[RFAv3 0/6] Implement | (pipe) command.

[RFAv3 0/6] Implement | (pipe) command.

[Philippe Waroquiers]

Implement | (pipe) command.

This patch series adds the pipe command, that allows to send the output
of a GDB command to a shell command.

This is the third version, handling the comments of Pedro.
The doc, help and NEWS are changed in this version, so must be
re-reviewed.

* Comments of Pedro:
  * Implement -d SEP (SEP being a string) instead of -dX.
  * popen has been kept as libiberty pexecute still implies to use
    the WIF* macros.  However, exit status handling reworked to go via
    convenience variables, rather than being shown to the user.
  * various small changes (== '\0', Skip, ...).
  * simplified error messages to just indicate with a static string
    what argument is wrong or missing.
  * extended the test to verify all error handling messages and check
    the new convenience variables $_shell_exitcode and $_shell_exitsignal.

* Comments from Eli :
   * better definition of WIF* macros for MinGW.
   * replace @ref by @xref in the doc.

* Comment from Abhijit Halder/Tom:
   * it not that unlikely to have | in a GDB command
     => an optional -d SEP option allows to specify an
     alternate string to use SEP to replace the | as separator
     between the GDB COMMAND and the SHELL_COMMAND.

* Comments from Tom:
  * make previous_saved_command_line static. For this, saved_command_line
    is now also static, and all repeat related functions/vars are now
    in top.c
  * various small changes (use std::swap, strchr, .empty (), ...).
  * removed the scoped_restore_current_thread restore
  * popen has been kept as libiberty pexecute still implies to use
    the WIF* macros.
  * Instead of using execute_command_to_string, use GDB redirection
    mechanism.  I did several trials for this, and at the end,
    the only one working properly was very close to the code
    of execute_command_to_string.
    => we now have a function execute_command_to_ui_file that is used
    to implement the pipe command, and also used by
    execute_command_to_string.

[RFAv3 1/6] Add previous_saved_command_line to allow a command to repeat a previous command.

[Philippe Waroquiers]

Currently, a previous command can be repeated when the user types an
empty line.  This is implemented in handle_line_of_input by
returning saved_command_line in case an empty line has been input.

If we want a command to repeat the previous command, we need to save
the previous saved_command_line, as when a command runs, the saved_command_line
already contains the current command line of the command being executed.

As suggested by Tom, the previous_saved_command_line is made static.
At the same time, saved_command_line is also made static.
The support functions/variables for the repeat command logic are now all
located inside top.c.

gdb/ChangeLog
2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* top.h (saved_command_line): Remove declaration.
	* top.c (previous_saved_command_line, previous_repeat_arguments):
	New variables.
	(saved_command_line): Make static, define together with other
	'repeat variables'.
	(dont_repeat): Clear repeat_arguments.
	(repeat_previous, get_saved_command_line, save_command_line):
	New functions.
	(gdb_init): Initialize saved_command_line
	and previous_saved_command_line.
	* main.c (captured_main_1): Remove saved_command_line initialization.
	* event-top.c (handle_line_of_input): Update to use
	the new 'repeat' related functions instead of direct access to
	saved_command_line.
	* command.h (repeat_previous, get_saved_command_line,
	save_command_line): New declarations.
	(dont_repeat): Add comment.
---
 gdb/command.h   | 36 ++++++++++++++++++++++++-
 gdb/event-top.c | 16 +++++------
 gdb/main.c      |  2 --
 gdb/top.c       | 70 +++++++++++++++++++++++++++++++++++++++++--------
 gdb/top.h       |  1 -
 5 files changed, 101 insertions(+), 24 deletions(-)

diff --git a/gdb/command.h b/gdb/command.h
index 4a239a7196..ea3a58f81c 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -448,7 +448,29 @@ extern void cmd_show_list (struct cmd_list_element *, int, const char *);
 
 extern void error_no_arg (const char *) ATTRIBUTE_NORETURN;
 
-extern void dont_repeat (void);
+
+/* Command line saving and repetition.
+   Each input line executed is saved for possibly be repeated either
+   when the user types an empty line, or be repeated by a command
+   that wants to repeat the previously executed command.  The below
+   functions are controlling command repetition.  */
+
+/* Commands call dont_repeat if they do not want to be repeated by null
+   lines or by repeat_previous ().  */
+
+extern void dont_repeat ();
+
+/* Command call repeat_previous if they want to repeat the previous command.
+   Such commands repeating the previous command must indicate
+   to not repeat themselves, to avoid recursive repeat.
+   repeat_previous will mark the current command as not repeating,
+   and will ensure get_saved_command_line returns the previous command,
+   so that the currently executing command can repeat it.  */
+
+extern void repeat_previous ();
+
+/* Prevent dont_repeat from working, and return a cleanup that
+   restores the previous state.  */
 
 extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
 
@@ -457,6 +479,18 @@ extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
 
 extern void set_repeat_arguments (const char *args);
 
+/* Returns the saved command line to repeat.
+   When a command is being executed, this is the currently executing
+   command line, unless the currently executing command has called
+   repeat_previous (): in this case, get_saved_command_line returns
+   the previously saved command line.  */
+
+extern char *get_saved_command_line ();
+
+/* Takes a copy of CMD, for possible repetition.  */
+
+extern void save_command_line (const char *cmd);
+
 /* Used to mark commands that don't do anything.  If we just leave the
    function field NULL, the command is interpreted as a help topic, or
    as a class of commands.  */
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3ccf136ff1..b64b7462c7 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -634,11 +634,10 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
    If REPEAT, handle command repetitions:
 
      - If the input command line is NOT empty, the command returned is
-       copied into the global 'saved_command_line' var so that it can
-       be repeated later.
+       saved using save_command_line () so that it can be repeated later.
 
-     - OTOH, if the input command line IS empty, return the previously
-       saved command instead of the empty input line.
+     - OTOH, if the input command line IS empty, return the saved
+       command instead of the empty input line.
 */
 
 char *
@@ -673,7 +672,7 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   server_command = startswith (cmd, SERVER_COMMAND_PREFIX);
   if (server_command)
     {
-      /* Note that we don't set `saved_command_line'.  Between this
+      /* Note that we don't call `save_command_line'.  Between this
          and the check in dont_repeat, this insures that repeating
          will still do the right thing.  */
       return cmd + strlen (SERVER_COMMAND_PREFIX);
@@ -713,7 +712,7 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
     ;
   if (repeat && *p1 == '\0')
-    return saved_command_line;
+    return get_saved_command_line ();
 
   /* Add command to history if appropriate.  Note: lines consisting
      solely of comments are also added to the command history.  This
@@ -728,9 +727,8 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   /* Save into global buffer if appropriate.  */
   if (repeat)
     {
-      xfree (saved_command_line);
-      saved_command_line = xstrdup (cmd);
-      return saved_command_line;
+      save_command_line (cmd);
+      return get_saved_command_line ();
     }
   else
     return cmd;
diff --git a/gdb/main.c b/gdb/main.c
index 35df1e497f..ef9bfe8fc6 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -499,8 +499,6 @@ captured_main_1 (struct captured_main_args *context)
 
   notice_open_fds ();
 
-  saved_command_line = (char *) xstrdup ("");
-
 #ifdef __MINGW32__
   /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
      as a Windows pipe, and Windows buffers on pipes.  */
diff --git a/gdb/top.c b/gdb/top.c
index bacd684dba..70f685ffad 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -134,8 +134,26 @@ show_confirm (struct ui_file *file, int from_tty,
 char *current_directory;
 
 /* The last command line executed on the console.  Used for command
-   repetitions.  */
-char *saved_command_line;
+   repetitions when the user enters an empty line.  */
+
+static char *saved_command_line;
+
+/* If not NULL, the arguments that should be passed if
+   saved_command_line is repeated.  */
+
+static const char *repeat_arguments;
+
+/* The previous last command line executed on the console.  Used for command
+   repetitions when a command want to relaunch the previously launched
+   command.  We need this as when a command is running, saved_command_line
+   already contains the line of the currently executing command.  */
+
+char *previous_saved_command_line;
+
+/* If not NULL, the arguments that should be passed if the
+   previous_saved_command_line is repeated.  */
+
+static const char *previous_repeat_arguments;
 
 /* Nonzero if the current command is modified by "server ".  This
    affects things like recording into the command history, commands
@@ -521,11 +539,6 @@ maybe_wait_sync_command_done (int was_sync)
     wait_sync_command_done ();
 }
 
-/* If not NULL, the arguments that should be passed if the current
-   command is repeated.  */
-
-static const char *repeat_arguments;
-
 /* See command.h.  */
 
 void
@@ -695,7 +708,7 @@ execute_command_to_string (const char *p, int from_tty,
 
 static int suppress_dont_repeat = 0;
 
-/* Commands call this if they do not want to be repeated by null lines.  */
+/* See command.h  */
 
 void
 dont_repeat (void)
@@ -709,11 +722,27 @@ dont_repeat (void)
      thing read from stdin in line and don't want to delete it.  Null
      lines won't repeat here in any case.  */
   if (ui->instream == ui->stdin_stream)
-    *saved_command_line = 0;
+    {
+      *saved_command_line = 0;
+      repeat_arguments = NULL;
+    }
+}
+
+/* See command.h  */
+
+void
+repeat_previous ()
+{
+  /* Do not repeat this command, as this command is a repeating command.  */
+  dont_repeat ();
+
+  /* We cannot free saved_command_line, as this line is being executed,
+     so swap it with previous_saved_command_line.  */
+  std::swap (previous_saved_command_line, saved_command_line);
+  std::swap (previous_repeat_arguments, repeat_arguments);
 }
 
-/* Prevent dont_repeat from working, and return a cleanup that
-   restores the previous state.  */
+/* See command.h.  */
 
 scoped_restore_tmpl<int>
 prevent_dont_repeat (void)
@@ -721,6 +750,22 @@ prevent_dont_repeat (void)
   return make_scoped_restore (&suppress_dont_repeat, 1);
 }
 
+char *
+get_saved_command_line ()
+{
+  return saved_command_line;
+}
+
+void
+save_command_line (const char *cmd)
+{
+  xfree (previous_saved_command_line);
+  previous_saved_command_line = saved_command_line;
+  previous_repeat_arguments = repeat_arguments;
+  saved_command_line = xstrdup (cmd);
+  repeat_arguments = NULL;
+}
+
 \f
 /* Read a line from the stream "instream" without command line editing.
 
@@ -2179,6 +2224,9 @@ The second argument is the terminal the UI runs on.\n"), &cmdlist);
 void
 gdb_init (char *argv0)
 {
+  saved_command_line = (char *) xstrdup ("");
+  previous_saved_command_line = (char *) xstrdup ("");
+
   if (pre_init_ui_hook)
     pre_init_ui_hook ();
 
diff --git a/gdb/top.h b/gdb/top.h
index 025d9389d6..aab03c13d6 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -217,7 +217,6 @@ extern void ui_register_input_event_handler (struct ui *ui);
 extern void ui_unregister_input_event_handler (struct ui *ui);
 
 /* From top.c.  */
-extern char *saved_command_line;
 extern int confirm;
 extern int inhibit_gdbinit;
 extern const char gdbinit[];
-- 
2.20.1

Re: [RFAv3 1/6] Add previous_saved_command_line to allow a command to repeat a previous command.

[Pedro Alves]

On 5/4/19 5:17 PM, Philippe Waroquiers wrote:
> Currently, a previous command can be repeated when the user types an
> empty line.  This is implemented in handle_line_of_input by
> returning saved_command_line in case an empty line has been input.
> 
> If we want a command to repeat the previous command, we need to save
> the previous saved_command_line, as when a command runs, the saved_command_line
> already contains the current command line of the command being executed.
> 
> As suggested by Tom, the previous_saved_command_line is made static.
> At the same time, saved_command_line is also made static.
> The support functions/variables for the repeat command logic are now all
> located inside top.c.
> 
> gdb/ChangeLog
> 2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* top.h (saved_command_line): Remove declaration.
> 	* top.c (previous_saved_command_line, previous_repeat_arguments):
> 	New variables.
> 	(saved_command_line): Make static, define together with other
> 	'repeat variables'.
> 	(dont_repeat): Clear repeat_arguments.
> 	(repeat_previous, get_saved_command_line, save_command_line):
> 	New functions.
> 	(gdb_init): Initialize saved_command_line
> 	and previous_saved_command_line.
> 	* main.c (captured_main_1): Remove saved_command_line initialization.
> 	* event-top.c (handle_line_of_input): Update to use
> 	the new 'repeat' related functions instead of direct access to
> 	saved_command_line.
> 	* command.h (repeat_previous, get_saved_command_line,
> 	save_command_line): New declarations.
> 	(dont_repeat): Add comment.
> ---
>  gdb/command.h   | 36 ++++++++++++++++++++++++-
>  gdb/event-top.c | 16 +++++------
>  gdb/main.c      |  2 --
>  gdb/top.c       | 70 +++++++++++++++++++++++++++++++++++++++++--------
>  gdb/top.h       |  1 -
>  5 files changed, 101 insertions(+), 24 deletions(-)
> 
> diff --git a/gdb/command.h b/gdb/command.h
> index 4a239a7196..ea3a58f81c 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -448,7 +448,29 @@ extern void cmd_show_list (struct cmd_list_element *, int, const char *);
>  
>  extern void error_no_arg (const char *) ATTRIBUTE_NORETURN;
>  
> -extern void dont_repeat (void);
> +
> +/* Command line saving and repetition.
> +   Each input line executed is saved for possibly be repeated either

s/saved for ... be/saved to ... be/

> +   when the user types an empty line, or be repeated by a command
> +   that wants to repeat the previously executed command.  The below
> +   functions are controlling command repetition.  */

s/are controlling/control/

> +
> +/* Commands call dont_repeat if they do not want to be repeated by null
> +   lines or by repeat_previous ().  */
> +
> +extern void dont_repeat ();
> +
> +/* Command call repeat_previous if they want to repeat the previous command.

s/Command call/Commands call/

> +   Such commands repeating the previous command must indicate

s/Such commands repeating/Such commands that repeat/

> +   to not repeat themselves, to avoid recursive repeat.
> +   repeat_previous will mark the current command as not repeating,
> +   and will ensure get_saved_command_line returns the previous command,
> +   so that the currently executing command can repeat it.  */
> +
> +extern void repeat_previous ();
> +
> +/* Prevent dont_repeat from working, and return a cleanup that
> +   restores the previous state.  */
>  
>  extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
>  
> @@ -457,6 +479,18 @@ extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
>  
>  extern void set_repeat_arguments (const char *args);
>  
> +/* Returns the saved command line to repeat.
> +   When a command is being executed, this is the currently executing
> +   command line, unless the currently executing command has called
> +   repeat_previous (): in this case, get_saved_command_line returns
> +   the previously saved command line.  */
> +
> +extern char *get_saved_command_line ();
> +
> +/* Takes a copy of CMD, for possible repetition.  */
> +
> +extern void save_command_line (const char *cmd);
> +
>  /* Used to mark commands that don't do anything.  If we just leave the
>     function field NULL, the command is interpreted as a help topic, or
>     as a class of commands.  */
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 3ccf136ff1..b64b7462c7 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -634,11 +634,10 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
>     If REPEAT, handle command repetitions:
>  
>       - If the input command line is NOT empty, the command returned is
> -       copied into the global 'saved_command_line' var so that it can
> -       be repeated later.
> +       saved using save_command_line () so that it can be repeated later.
>  
> -     - OTOH, if the input command line IS empty, return the previously
> -       saved command instead of the empty input line.
> +     - OTOH, if the input command line IS empty, return the saved
> +       command instead of the empty input line.
>  */
>  
>  char *
> @@ -673,7 +672,7 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
>    server_command = startswith (cmd, SERVER_COMMAND_PREFIX);
>    if (server_command)
>      {
> -      /* Note that we don't set `saved_command_line'.  Between this
> +      /* Note that we don't call `save_command_line'.  Between this
>           and the check in dont_repeat, this insures that repeating
>           will still do the right thing.  */
>        return cmd + strlen (SERVER_COMMAND_PREFIX);
> @@ -713,7 +712,7 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
>    for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
>      ;
>    if (repeat && *p1 == '\0')
> -    return saved_command_line;
> +    return get_saved_command_line ();
>  
>    /* Add command to history if appropriate.  Note: lines consisting
>       solely of comments are also added to the command history.  This
> @@ -728,9 +727,8 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
>    /* Save into global buffer if appropriate.  */
>    if (repeat)
>      {
> -      xfree (saved_command_line);
> -      saved_command_line = xstrdup (cmd);
> -      return saved_command_line;
> +      save_command_line (cmd);
> +      return get_saved_command_line ();
>      }
>    else
>      return cmd;
> diff --git a/gdb/main.c b/gdb/main.c
> index 35df1e497f..ef9bfe8fc6 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -499,8 +499,6 @@ captured_main_1 (struct captured_main_args *context)
>  
>    notice_open_fds ();
>  
> -  saved_command_line = (char *) xstrdup ("");
> -
>  #ifdef __MINGW32__
>    /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
>       as a Windows pipe, and Windows buffers on pipes.  */
> diff --git a/gdb/top.c b/gdb/top.c
> index bacd684dba..70f685ffad 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -134,8 +134,26 @@ show_confirm (struct ui_file *file, int from_tty,
>  char *current_directory;
>  
>  /* The last command line executed on the console.  Used for command
> -   repetitions.  */
> -char *saved_command_line;
> +   repetitions when the user enters an empty line.  */
> +
> +static char *saved_command_line;
> +
> +/* If not NULL, the arguments that should be passed if
> +   saved_command_line is repeated.  */
> +
> +static const char *repeat_arguments;
> +
> +/* The previous last command line executed on the console.  Used for command
> +   repetitions when a command want to relaunch the previously launched

s/a command want/a command wants/

> +   command.  We need this as when a command is running, saved_command_line
> +   already contains the line of the currently executing command.  */
> +
> +char *previous_saved_command_line;
> +
> +/* If not NULL, the arguments that should be passed if the
> +   previous_saved_command_line is repeated.  */
> +
> +static const char *previous_repeat_arguments;
>  
>  /* Nonzero if the current command is modified by "server ".  This
>     affects things like recording into the command history, commands
> @@ -521,11 +539,6 @@ maybe_wait_sync_command_done (int was_sync)
>      wait_sync_command_done ();
>  }
>  
> -/* If not NULL, the arguments that should be passed if the current
> -   command is repeated.  */
> -
> -static const char *repeat_arguments;
> -
>  /* See command.h.  */
>  
>  void
> @@ -695,7 +708,7 @@ execute_command_to_string (const char *p, int from_tty,
>  
>  static int suppress_dont_repeat = 0;
>  
> -/* Commands call this if they do not want to be repeated by null lines.  */
> +/* See command.h  */
>  
>  void
>  dont_repeat (void)
> @@ -709,11 +722,27 @@ dont_repeat (void)
>       thing read from stdin in line and don't want to delete it.  Null
>       lines won't repeat here in any case.  */
>    if (ui->instream == ui->stdin_stream)
> -    *saved_command_line = 0;
> +    {
> +      *saved_command_line = 0;
> +      repeat_arguments = NULL;
> +    }
> +}
> +
> +/* See command.h  */
> +
> +void
> +repeat_previous ()
> +{
> +  /* Do not repeat this command, as this command is a repeating command.  */
> +  dont_repeat ();
> +
> +  /* We cannot free saved_command_line, as this line is being executed,
> +     so swap it with previous_saved_command_line.  */
> +  std::swap (previous_saved_command_line, saved_command_line);
> +  std::swap (previous_repeat_arguments, repeat_arguments);
>  }
>  
> -/* Prevent dont_repeat from working, and return a cleanup that
> -   restores the previous state.  */
> +/* See command.h.  */
>  
>  scoped_restore_tmpl<int>
>  prevent_dont_repeat (void)
> @@ -721,6 +750,22 @@ prevent_dont_repeat (void)
>    return make_scoped_restore (&suppress_dont_repeat, 1);
>  }
>  
> +char *
> +get_saved_command_line ()

Add usual /* See foo.h  */ comment.


> +{
> +  return saved_command_line;
> +}
> +
> +void
> +save_command_line (const char *cmd)

Ditto.

> +{
> +  xfree (previous_saved_command_line);
> +  previous_saved_command_line = saved_command_line;
> +  previous_repeat_arguments = repeat_arguments;
> +  saved_command_line = xstrdup (cmd);
> +  repeat_arguments = NULL;
> +}
> +
>  \f
>  /* Read a line from the stream "instream" without command line editing.
>  
> @@ -2179,6 +2224,9 @@ The second argument is the terminal the UI runs on.\n"), &cmdlist);
>  void
>  gdb_init (char *argv0)
>  {
> +  saved_command_line = (char *) xstrdup ("");
> +  previous_saved_command_line = (char *) xstrdup ("");

Drop the unnecessary casts.

> +
>    if (pre_init_ui_hook)
>      pre_init_ui_hook ();
>  
> diff --git a/gdb/top.h b/gdb/top.h
> index 025d9389d6..aab03c13d6 100644
> --- a/gdb/top.h
> +++ b/gdb/top.h
> @@ -217,7 +217,6 @@ extern void ui_register_input_event_handler (struct ui *ui);
>  extern void ui_unregister_input_event_handler (struct ui *ui);
>  
>  /* From top.c.  */
> -extern char *saved_command_line;
>  extern int confirm;
>  extern int inhibit_gdbinit;
>  extern const char gdbinit[];
> 
Thanks,
Pedro Alves

[RFAv3 6/6] NEWS and documentation for | (pipe) command.

[Philippe Waroquiers]

gdb/ChangeLog
	* NEWS: Mention new pipe command and new convenience variables.

gdb/doc/ChangeLog
	* gdb.texinfo (Shell Commands): Document pipe command.
	(Logging Output): Add a reference to pipe command.
	(Convenience Variables): Document $_shell_exitcode and
	$_shell_exitstatus.
---
 gdb/NEWS            | 32 ++++++++++++++--------
 gdb/doc/gdb.texinfo | 67 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 11 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b21b2cbb47..570718ab5c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -16,11 +16,23 @@
 
 * Support for Pointer Authentication on AArch64 Linux.
 
-* Two new convernience functions $_cimag and $_creal that extract the
+* Two new convenience functions $_cimag and $_creal that extract the
   imaginary and real parts respectively from complex numbers.
 
+* New built-in convenience variables $_shell_exitcode and $_shell_exitsignal
+  provide the exitcode or exit status of the shell commands launched by
+  GDB commands such as "shell", "pipe", "make".
+
 * New commands
 
+pipe [COMMAND] | SHELL_COMMAND
+| [COMMAND] | SHELL_COMMAND
+pipe -d SEP COMMAND SEP SHELL_COMMAND
+| -s DEP COMMAND SEP SHELL_COMMAND
+  Executes COMMAND and sends its output to SHELL_COMMAND.
+  With no COMMAND, repeat the last executed command
+  and send its output to SHELL_COMMAND.
+
 set print max-depth
 show print max-depth
   Allows deeply nested structures to be simplified when printing by
@@ -28,16 +40,6 @@ show print max-depth
   The default max-depth is 20, but this can be set to unlimited to get
   the old behavior back.
 
-* Python API
-
-  ** The gdb.Value type has a new method 'format_string' which returns a
-     string representing the value.  The formatting is controlled by the
-     optional keyword arguments: 'raw', 'pretty_arrays', 'pretty_structs',
-     'array_indexes', 'symbols', 'unions', 'deref_refs', 'actual_objects',
-     'static_members', 'max_elements', 'repeat_threshold', and 'format'.
-
-* New commands
-
 set may-call-functions [on|off]
 show may-call-functions
   This controls whether GDB will attempt to call functions in
@@ -48,6 +50,14 @@ show may-call-functions
   an error when a command (such as print expression) calls a function
   in the program.
 
+* Python API
+
+  ** The gdb.Value type has a new method 'format_string' which returns a
+     string representing the value.  The formatting is controlled by the
+     optional keyword arguments: 'raw', 'pretty_arrays', 'pretty_structs',
+     'array_indexes', 'symbols', 'unions', 'deref_refs', 'actual_objects',
+     'static_members', 'max_elements', 'repeat_threshold', and 'format'.
+
 *** Changes in GDB 8.3
 
 * GDB and GDBserver now support access to additional registers on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index dd8ae91b93..36e3f8bc35 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1454,6 +1454,56 @@ Execute the @code{make} program with the specified
 arguments.  This is equivalent to @samp{shell make @var{make-args}}.
 @end table
 
+@table @code
+@kindex pipe
+@kindex |
+@cindex send the output of a gdb command to a shell command
+@anchor{pipe}
+@item pipe [@var{command}] | @var{shell_command}
+@itemx | [@var{command}] | @var{shell_command}
+@itemx pipe -d @var{sep} @var{command} @var{sep} @var{shell_command}
+@itemx | -d @var{sep} @var{command} @var{sep} @var{shell_command}
+Executes @var{command} and sends its output to @var{shell_command}.
+Note that no space is needed around @code{|}.
+If no @var{command} is provided, the last command executed is repeated.
+
+If the @var{command} contains a @code{|}, then the option @code{-d @var{sep}}
+can be used to specify an alternate separator string @var{sep} that separates
+the  @var{command} from the @var{shell_command}.
+
+Example:
+@smallexample
+@group
+(gdb) pipe p full|wc
+      5      17      81
+(gdb) |p full|wc -l
+5
+@end group
+@group
+(gdb) p full
+$4 = (black => 144,
+  red => 233,
+  green => 377,
+  blue => 610,
+  white => 987)
+(gdb) ||grep red
+  red => 233,
+@end group
+@group
+(gdb) | -d ! echo this contains a | char\n ! sed -e 's/|/PIPE/'
+this contains a PIPE char
+(gdb) | -d xxx echo this contains a | char!\n xxx sed -e 's/|/PIPE/'
+this contains a PIPE char!
+(gdb)
+@end group
+@end smallexample
+@end table
+
+The convenience variables @code{$_shell_exitcode} and @code{$_shell_exitsignal}
+can be used to examine the exit status of the last shell command launched
+by @code{shell}, @code{make}, @code{pipe} and @code{|}.
+@xref{Convenience Vars,, Convenience Variables}.
+
 @node Logging Output
 @section Logging Output
 @cindex logging @value{GDBN} output
@@ -1482,6 +1532,8 @@ Set @code{redirect} if you want output to go only to the log file.
 Show the current values of the logging settings.
 @end table
 
+You can also redirect the output of a @value{GDBN} command to a
+shell command.  @xref{pipe}.
 @node Commands
 @chapter @value{GDBN} Commands
 
@@ -11292,6 +11344,21 @@ the value 12 for @code{$_gdb_minor}.  These variables allow you to
 write scripts that work with different versions of @value{GDBN}
 without errors caused by features unavailable in some of those
 versions.
+
+@item $_shell_exitcode
+@itemx $_shell_exitsignal
+@vindex $_shell_exitcode@r{, convenience variable}
+@vindex $_shell_exitsignal@r{, convenience variable}
+@cindex shell command, exit code
+@cindex shell command, exit signal
+@cindex exit status of shell commands
+@value{GDBN} commands such as @code{shell}, @code{|} are launching
+shell commands.  When a launched command terminates, @value{GDBN}
+automatically maintains the variables @code{$_shell_exitcode}
+and @code{$_shell_exitsignal} according to the exit status of the last
+launched command.  These variables are set and used similarly to
+the variables @code{$_exitcode} and @code{$_exitsignal}.
+
 @end table
 
 @node Convenience Funs
-- 
2.20.1

Re: [RFAv3 6/6] NEWS and documentation for | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat,  4 May 2019 18:17:53 +0200
> 
> gdb/ChangeLog
> 	* NEWS: Mention new pipe command and new convenience variables.
> 
> gdb/doc/ChangeLog
> 	* gdb.texinfo (Shell Commands): Document pipe command.
> 	(Logging Output): Add a reference to pipe command.
> 	(Convenience Variables): Document $_shell_exitcode and
> 	$_shell_exitstatus.

The document part is OK.

Thanks.

Re: [RFAv3 6/6] NEWS and documentation for | (pipe) command.

[Eli Zaretskii]

> Date: Sat, 04 May 2019 19:26:10 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: gdb-patches@sourceware.org
> 
> The document part is OK.
      ^^^^^^^^
I meant "documentation", of course.  Sorry.

Re: [RFAv3 6/6] NEWS and documentation for | (pipe) command.

[Pedro Alves]

On 5/4/19 5:17 PM, Philippe Waroquiers wrote:
> gdb/ChangeLog
> 	* NEWS: Mention new pipe command and new convenience variables.
> 
> gdb/doc/ChangeLog
> 	* gdb.texinfo (Shell Commands): Document pipe command.
> 	(Logging Output): Add a reference to pipe command.
> 	(Convenience Variables): Document $_shell_exitcode and
> 	$_shell_exitstatus.
> ---
>  gdb/NEWS            | 32 ++++++++++++++--------
>  gdb/doc/gdb.texinfo | 67 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index b21b2cbb47..570718ab5c 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -16,11 +16,23 @@
>  
>  * Support for Pointer Authentication on AArch64 Linux.
>  
> -* Two new convernience functions $_cimag and $_creal that extract the
> +* Two new convenience functions $_cimag and $_creal that extract the
>    imaginary and real parts respectively from complex numbers.
>  

Please go ahead and push this bit in as an obvious fix.

> +* New built-in convenience variables $_shell_exitcode and $_shell_exitsignal
> +  provide the exitcode or exit status of the shell commands launched by
> +  GDB commands such as "shell", "pipe", "make".
> +

That "make" should be preceded by "and".

(I'll leave oxford comma or not to someone else.  :-) )

>  * New commands
>  
> +pipe [COMMAND] | SHELL_COMMAND
> +| [COMMAND] | SHELL_COMMAND
> +pipe -d SEP COMMAND SEP SHELL_COMMAND
> +| -s DEP COMMAND SEP SHELL_COMMAND

s/DEP/SEP/

> +  Executes COMMAND and sends its output to SHELL_COMMAND.
> +  With no COMMAND, repeat the last executed command
> +  and send its output to SHELL_COMMAND.
> +
>  set print max-depth
>  show print max-depth
>    Allows deeply nested structures to be simplified when printing by
> @@ -28,16 +40,6 @@ show print max-depth
>    The default max-depth is 20, but this can be set to unlimited to get
>    the old behavior back.
>  
> -* Python API
> -
> -  ** The gdb.Value type has a new method 'format_string' which returns a
> -     string representing the value.  The formatting is controlled by the
> -     optional keyword arguments: 'raw', 'pretty_arrays', 'pretty_structs',
> -     'array_indexes', 'symbols', 'unions', 'deref_refs', 'actual_objects',
> -     'static_members', 'max_elements', 'repeat_threshold', and 'format'.
> -
> -* New commands
> -
>  set may-call-functions [on|off]
>  show may-call-functions
>    This controls whether GDB will attempt to call functions in
> @@ -48,6 +50,14 @@ show may-call-functions
>    an error when a command (such as print expression) calls a function
>    in the program.
>  
> +* Python API
> +
> +  ** The gdb.Value type has a new method 'format_string' which returns a
> +     string representing the value.  The formatting is controlled by the
> +     optional keyword arguments: 'raw', 'pretty_arrays', 'pretty_structs',
> +     'array_indexes', 'symbols', 'unions', 'deref_refs', 'actual_objects',
> +     'static_members', 'max_elements', 'repeat_threshold', and 'format'.
> +

This also looks like a change that should go in separately, as
an obvious fix.

>  *** Changes in GDB 8.3
>  
>  * GDB and GDBserver now support access to additional registers on
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index dd8ae91b93..36e3f8bc35 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1454,6 +1454,56 @@ Execute the @code{make} program with the specified
>  arguments.  This is equivalent to @samp{shell make @var{make-args}}.
>  @end table
>  
> +@table @code
> +@kindex pipe
> +@kindex |
> +@cindex send the output of a gdb command to a shell command
> +@anchor{pipe}
> +@item pipe [@var{command}] | @var{shell_command}
> +@itemx | [@var{command}] | @var{shell_command}
> +@itemx pipe -d @var{sep} @var{command} @var{sep} @var{shell_command}
> +@itemx | -d @var{sep} @var{command} @var{sep} @var{shell_command}
> +Executes @var{command} and sends its output to @var{shell_command}.
> +Note that no space is needed around @code{|}.
> +If no @var{command} is provided, the last command executed is repeated.
> +
> +If the @var{command} contains a @code{|}, then the option @code{-d @var{sep}}
> +can be used to specify an alternate separator string @var{sep} that separates
> +the  @var{command} from the @var{shell_command}.
> +

(spurious double space after "the" in the last line.)

Like in the "help" output, I think we can improve this a bit with a slight
copy/edit.  "If contains |, then" suggests that you can only use -d
if the command contains "|", which is obviously not really the intention.

I'd suggest:

 In case the @var{command} contains a @code{|}, the option @code{-d @var{sep}}
 can be used to specify an alternate separator string @var{sep} that separates
 the @var{command} from the @var{shell_command}.

"in case" suggests a precautionary measure.

> +Example:
> +@smallexample
> +@group
> +(gdb) pipe p full|wc
> +      5      17      81
> +(gdb) |p full|wc -l
> +5
> +@end group
> +@group
> +(gdb) p full
> +$4 = (black => 144,
> +  red => 233,
> +  green => 377,
> +  blue => 610,
> +  white => 987)
> +(gdb) ||grep red
> +  red => 233,

I'd suggest printing "p full" before the pipe commands.
I read this top to bottom and it took me a second to realize
what "full" was.  Maybe rename it to "var" or something like that,
while at it.

Use "$1" for value history number.

( I guess that's Ada syntax.  I'd hazard a guess that a significant
number of GDB users don't know Ada.  :-) )

> +@end group
> +@group
> +(gdb) | -d ! echo this contains a | char\n ! sed -e 's/|/PIPE/'
> +this contains a PIPE char
> +(gdb) | -d xxx echo this contains a | char!\n xxx sed -e 's/|/PIPE/'
> +this contains a PIPE char!
> +(gdb)
> +@end group
> +@end smallexample
> +@end table
> +
> +The convenience variables @code{$_shell_exitcode} and @code{$_shell_exitsignal}
> +can be used to examine the exit status of the last shell command launched
> +by @code{shell}, @code{make}, @code{pipe} and @code{|}.
> +@xref{Convenience Vars,, Convenience Variables}.
> +
>  @node Logging Output
>  @section Logging Output
>  @cindex logging @value{GDBN} output
> @@ -1482,6 +1532,8 @@ Set @code{redirect} if you want output to go only to the log file.
>  Show the current values of the logging settings.
>  @end table
>  
> +You can also redirect the output of a @value{GDBN} command to a
> +shell command.  @xref{pipe}.
>  @node Commands
>  @chapter @value{GDBN} Commands
>  
> @@ -11292,6 +11344,21 @@ the value 12 for @code{$_gdb_minor}.  These variables allow you to
>  write scripts that work with different versions of @value{GDBN}
>  without errors caused by features unavailable in some of those
>  versions.
> +
> +@item $_shell_exitcode
> +@itemx $_shell_exitsignal
> +@vindex $_shell_exitcode@r{, convenience variable}
> +@vindex $_shell_exitsignal@r{, convenience variable}
> +@cindex shell command, exit code
> +@cindex shell command, exit signal
> +@cindex exit status of shell commands
> +@value{GDBN} commands such as @code{shell}, @code{|} are launching

List of two items -> use "and", not ",", thus:

   @code{shell} and @code{|}

> +shell commands.  When a launched command terminates, @value{GDBN}
> +automatically maintains the variables @code{$_shell_exitcode}
> +and @code{$_shell_exitsignal} according to the exit status of the last
> +launched command.  These variables are set and used similarly to
> +the variables @code{$_exitcode} and @code{$_exitsignal}.
> +
>  @end table
>  
>  @node Convenience Funs
> 

Thanks,
Pedro Alves

[RFAv3 3/6] Add function execute_command_to_ui_file

[Philippe Waroquiers]

2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdbcmd.h (execute_command_to_ui_file): New declaration.
	top.c (execute_command_to_ui_file): New function, mostly a copy
	of execute_command_to_string.
	(execute_command_to_string): Implement by calling
	execute_command_to_ui_file.
---
 gdb/gdbcmd.h |  2 ++
 gdb/top.c    | 35 ++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
index 5d0e697d83..1b47719f18 100644
--- a/gdb/gdbcmd.h
+++ b/gdb/gdbcmd.h
@@ -139,6 +139,8 @@ extern void execute_command (const char *, int);
    as cli_styling.  */
 extern std::string execute_command_to_string (const char *p, int from_tty,
 					      bool term_out);
+extern void execute_command_to_ui_file (struct ui_file *file,
+					const char *p, int from_tty);
 
 extern void print_command_line (struct command_line *, unsigned int,
 				struct ui_file *);
diff --git a/gdb/top.c b/gdb/top.c
index 70f685ffad..900c3fc3e2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -665,13 +665,12 @@ execute_command (const char *p, int from_tty)
   cleanup_if_error.release ();
 }
 
-/* Run execute_command for P and FROM_TTY.  Capture its output into the
-   returned string, do not display it to the screen.  BATCH_FLAG will be
+/* Run execute_command for P and FROM_TTY.  Sends its output to FILE,
+   do not display it to the screen.  BATCH_FLAG will be
    temporarily set to true.  */
 
-std::string
-execute_command_to_string (const char *p, int from_tty,
-			   bool term_out)
+void
+execute_command_to_ui_file (struct ui_file *file, const char *p, int from_tty)
 {
   /* GDB_STDOUT should be better already restored during these
      restoration callbacks.  */
@@ -679,26 +678,36 @@ execute_command_to_string (const char *p, int from_tty,
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
-  string_file str_file (term_out);
-
   {
-    current_uiout->redirect (&str_file);
+    current_uiout->redirect (file);
     ui_out_redirect_pop redirect_popper (current_uiout);
 
     scoped_restore save_stdout
-      = make_scoped_restore (&gdb_stdout, &str_file);
+      = make_scoped_restore (&gdb_stdout, file);
     scoped_restore save_stderr
-      = make_scoped_restore (&gdb_stderr, &str_file);
+      = make_scoped_restore (&gdb_stderr, file);
     scoped_restore save_stdlog
-      = make_scoped_restore (&gdb_stdlog, &str_file);
+      = make_scoped_restore (&gdb_stdlog, file);
     scoped_restore save_stdtarg
-      = make_scoped_restore (&gdb_stdtarg, &str_file);
+      = make_scoped_restore (&gdb_stdtarg, file);
     scoped_restore save_stdtargerr
-      = make_scoped_restore (&gdb_stdtargerr, &str_file);
+      = make_scoped_restore (&gdb_stdtargerr, file);
 
     execute_command (p, from_tty);
   }
+}
+
+/* Run execute_command for P and FROM_TTY.  Capture its output into the
+   returned string, do not display it to the screen.  BATCH_FLAG will be
+   temporarily set to true.  */
+
+std::string
+execute_command_to_string (const char *p, int from_tty,
+			   bool term_out)
+{
+  string_file str_file (term_out);
 
+  execute_command_to_ui_file (&str_file, p, from_tty);
   return std::move (str_file.string ());
 }
 
-- 
2.20.1

[RFAv3 5/6] Test the | (pipe) command.

[Philippe Waroquiers]

gdb/testsuite/ChangeLog
2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/shell.exp: Test pipe command, $_shell_exitcode,
	$_shell_exitsignal.
	* gdb.base/default.exp: Update for new convenience variables.

Update default.exp
---
 gdb/testsuite/gdb.base/default.exp |  2 +
 gdb/testsuite/gdb.base/shell.exp   | 68 +++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 56ec917aa3..0325b8045d 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -606,6 +606,8 @@ set show_conv_list \
 	{$_isvoid = <internal function _isvoid>} \
 	{$_gdb_major = 8} \
 	{$_gdb_minor = 4} \
+	{$_shell_exitsignal = void} \
+	{$_shell_exitcode = 0} \
     }
 if ![skip_python_tests] {
     append show_conv_list \
diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 60d6e31e4f..9b85e4988b 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Test that the shell and ! commands work.
+# Test that the shell and ! and | and pipe commands work.
 
 gdb_exit
 gdb_start
@@ -22,3 +22,69 @@ gdb_test "shell echo foo" "foo"
 
 gdb_test "! echo foo" "foo"
 gdb_test "!echo foo" "foo"
+
+# Convenience variables with shell command.
+gdb_test "! exit 0" ""
+gdb_test "p \$_shell_exitcode" " = 0" "shell success exitcode"
+gdb_test "p \$_shell_exitsignal" " = void" "shell success exitsignal"
+
+gdb_test "! exit 1" ""
+gdb_test "p \$_shell_exitcode" " = 1" "shell fail exitcode"
+gdb_test "p \$_shell_exitsignal" " = void" "shell fail exitsignal"
+
+gdb_test "! kill -2 $$" ""
+gdb_test "p \$_shell_exitcode" " = void" "shell interrupt exitcode"
+gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
+
+
+gdb_test "pipe help pipe | wc -l" "10" "check simple pipe"
+gdb_test "pipe help pipe | grep Usage: | wc -l" "4" "check double pipe"
+
+gdb_test "| help pipe | grep Usage: | wc -l" "4" "check double pipe, pipe char"
+gdb_test "|help pipe|grep Usage:|wc -l" "4" "no space around pipe char"
+
+gdb_test "echo coucou\\n" "coucou" "echo coucou"
+gdb_test "||wc -l" "1" "Check repeat previous command"
+
+gdb_test "| -d ! echo this contains a | character\\n ! sed -e 's/|/PIPE/'" \
+    "this contains a PIPE character" "verify alternate 1char sep"
+
+gdb_test "|-d ! echo this contains a | character\\n!sed -e 's/|/PIPE/'" \
+    "this contains a PIPE character" "verify alternate 1char sep, no space"
+
+gdb_test "| -d !!! echo this contains a | character\\n !!! sed -e 's/|/PIPE/'" \
+    "this contains a PIPE character" "verify alternate 3char sep"
+
+gdb_test "|-d !!! echo this contains a | character\\n!!!sed -e 's/|/PIPE/'" \
+    "this contains a PIPE character" "verify alternate 3char sep, no space"
+
+# Convenience variables with pipe command.
+gdb_test "|p 123| exit 0" ""
+gdb_test "p \$_shell_exitcode" " = 0" "pipe success exitcode"
+gdb_test "p \$_shell_exitsignal" " = void" "pipe success exitsignal"
+
+gdb_test "|p 123| exit 1" ""
+gdb_test "p \$_shell_exitcode" " = 1" "pipe fail exitcode"
+gdb_test "p \$_shell_exitsignal" " = void" "pipe fail exitsignal"
+
+gdb_test "|p 123| kill -2 $$" ""
+gdb_test "p \$_shell_exitcode" " = void" "pipe interrupt exitcode"
+gdb_test "p \$_shell_exitsignal" " = 2" "pipe interrupt exitsignal"
+
+# Error handling verifications.
+gdb_test "|" "Missing COMMAND" "all missing"
+gdb_test "|-d" "Missing separator SEP after -d" "-d value missing"
+gdb_test "|-d    " "Missing separator SEP after -d" "-d spaces value missing"
+gdb_test "| echo coucou" \
+    "Missing separator before SHELL_COMMAND" \
+    "| separator missing"
+gdb_test "|-d SEP echo coucou" \
+    "Missing separator before SHELL_COMMAND" \
+    "SEP separator missing"
+gdb_test "|echo coucou|" \
+    "Missing SHELL_COMMAND" \
+    "SHELL_COMMAND missing"
+gdb_test "|-d ! echo coucou !" \
+    "Missing SHELL_COMMAND" \
+    "SHELL_COMMAND missing"
+
-- 
2.20.1

Re: [RFAv3 5/6] Test the | (pipe) command.

[Pedro Alves]

On 5/4/19 5:17 PM, Philippe Waroquiers wrote:
> gdb/testsuite/ChangeLog
> 2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/shell.exp: Test pipe command, $_shell_exitcode,
> 	$_shell_exitsignal.
> 	* gdb.base/default.exp: Update for new convenience variables.
> 
> Update default.exp

  ^^^^^^^^^^^^^^^^^^  spurious.

> ---
>  gdb/testsuite/gdb.base/default.exp |  2 +
>  gdb/testsuite/gdb.base/shell.exp   | 68 +++++++++++++++++++++++++++++-
>  2 files changed, 69 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
> index 56ec917aa3..0325b8045d 100644
> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -606,6 +606,8 @@ set show_conv_list \
>  	{$_isvoid = <internal function _isvoid>} \
>  	{$_gdb_major = 8} \
>  	{$_gdb_minor = 4} \
> +	{$_shell_exitsignal = void} \
> +	{$_shell_exitcode = 0} \
>      }
>  if ![skip_python_tests] {
>      append show_conv_list \
> diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
> index 60d6e31e4f..9b85e4988b 100644
> --- a/gdb/testsuite/gdb.base/shell.exp
> +++ b/gdb/testsuite/gdb.base/shell.exp
> @@ -13,7 +13,7 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -# Test that the shell and ! commands work.
> +# Test that the shell and ! and | and pipe commands work.

Too many ands.  I'd suggest:

# Test that the "shell", "!", "|" and "pipe" commands work.


>  
>  gdb_exit
>  gdb_start
> @@ -22,3 +22,69 @@ gdb_test "shell echo foo" "foo"
>  
>  gdb_test "! echo foo" "foo"
>  gdb_test "!echo foo" "foo"
> +
> +# Convenience variables with shell command.
> +gdb_test "! exit 0" ""

This always passes regardless of output.  Is that intended?
If not, use gdb_test_no_output.  Ditto for all other similar
cases below.

> +gdb_test "p \$_shell_exitcode" " = 0" "shell success exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "shell success exitsignal"
> +
> +gdb_test "! exit 1" ""
> +gdb_test "p \$_shell_exitcode" " = 1" "shell fail exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "shell fail exitsignal"
> +
> +gdb_test "! kill -2 $$" ""
> +gdb_test "p \$_shell_exitcode" " = void" "shell interrupt exitcode"
> +gdb_test "p \$_shell_exitsignal" " = 2" "shell interrupt exitsignal"
> +
> +
> +gdb_test "pipe help pipe | wc -l" "10" "check simple pipe"
> +gdb_test "pipe help pipe | grep Usage: | wc -l" "4" "check double pipe"

That looks a bit brittle.  We'll have to update the tests if we change the
help output.  Wouldn't something a bit more stable be better?  E.g.,
gdb has output/printf/echo commands.  You could use those to precisely control
the output in the testcase.  To avoid cluttering the test logs, you could
make the testcase define a command that uses echo, and then use that command:

define foo
  echo "hello world\n"
  ...
end

> +
> +gdb_test "| help pipe | grep Usage: | wc -l" "4" "check double pipe, pipe char"
> +gdb_test "|help pipe|grep Usage:|wc -l" "4" "no space around pipe char"
> +
> +gdb_test "echo coucou\\n" "coucou" "echo coucou"
> +gdb_test "||wc -l" "1" "Check repeat previous command"

Lowercase "Check".  Or better, remove "Check".

> +
> +gdb_test "| -d ! echo this contains a | character\\n ! sed -e 's/|/PIPE/'" \
> +    "this contains a PIPE character" "verify alternate 1char sep"
> +
> +gdb_test "|-d ! echo this contains a | character\\n!sed -e 's/|/PIPE/'" \
> +    "this contains a PIPE character" "verify alternate 1char sep, no space"
> +
> +gdb_test "| -d !!! echo this contains a | character\\n !!! sed -e 's/|/PIPE/'" \
> +    "this contains a PIPE character" "verify alternate 3char sep"
> +
> +gdb_test "|-d !!! echo this contains a | character\\n!!!sed -e 's/|/PIPE/'" \
> +    "this contains a PIPE character" "verify alternate 3char sep, no space"
> +
> +# Convenience variables with pipe command.
> +gdb_test "|p 123| exit 0" ""
> +gdb_test "p \$_shell_exitcode" " = 0" "pipe success exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "pipe success exitsignal"
> +
> +gdb_test "|p 123| exit 1" ""
> +gdb_test "p \$_shell_exitcode" " = 1" "pipe fail exitcode"
> +gdb_test "p \$_shell_exitsignal" " = void" "pipe fail exitsignal"
> +
> +gdb_test "|p 123| kill -2 $$" ""
> +gdb_test "p \$_shell_exitcode" " = void" "pipe interrupt exitcode"
> +gdb_test "p \$_shell_exitsignal" " = 2" "pipe interrupt exitsignal"
> +
> +# Error handling verifications.
> +gdb_test "|" "Missing COMMAND" "all missing"
> +gdb_test "|-d" "Missing separator SEP after -d" "-d value missing"
> +gdb_test "|-d    " "Missing separator SEP after -d" "-d spaces value missing"
> +gdb_test "| echo coucou" \
> +    "Missing separator before SHELL_COMMAND" \
> +    "| separator missing"
> +gdb_test "|-d SEP echo coucou" \
> +    "Missing separator before SHELL_COMMAND" \
> +    "SEP separator missing"
> +gdb_test "|echo coucou|" \
> +    "Missing SHELL_COMMAND" \
> +    "SHELL_COMMAND missing"
> +gdb_test "|-d ! echo coucou !" \
> +    "Missing SHELL_COMMAND" \
> +    "SHELL_COMMAND missing"
> +

At least the last two above have duplicate test names.

I think you're missing a test for "-dSEP", no space.

Thanks,
Pedro Alves

[RFAv3 2/6] Improve process exit status macros on MinGW

[Philippe Waroquiers]

gdb/ChangeLog
2019-05-04  Eli Zaretskii  <eliz@gnu.org>
	    Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* common/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS,
	WTERMSIG): Define better versions for MinGW.
	* windows-nat.c (xlate): Uncomment the definition.
	(windows_status_to_termsig): New function.
---
 gdb/common/gdb_wait.h | 27 +++++++++++++++++++++++++++
 gdb/windows-nat.c     | 18 ++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/gdb/common/gdb_wait.h b/gdb/common/gdb_wait.h
index b3b752cf3a..ca95240009 100644
--- a/gdb/common/gdb_wait.h
+++ b/gdb/common/gdb_wait.h
@@ -40,13 +40,31 @@
    NOTE exception for GNU/Linux below).  We also fail to declare
    wait() and waitpid().  */
 
+/* For MINGW, the underlying idea is that when a Windows program is terminated
+   by a fatal exception, its exit code is the value of that exception, as
+   defined by the various STATUS_* symbols in the Windows API headers.
+
+   The below is not perfect, because a program could legitimately exit normally
+   with a status whose value happens to have the high bits set, but that's
+   extremely rare, to say the least, and it is deemed such a negligibly small
+   probability of false positives is justified by the utility of reporting the
+   terminating signal in the "normal" cases.  */
+
 #ifndef	WIFEXITED
+#if defined (__MINGW32__)
+#define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
+#else
 #define WIFEXITED(w)	(((w)&0377) == 0)
 #endif
+#endif
 
 #ifndef	WIFSIGNALED
+#if defined (__MINGW32__)
+#define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) == 0xC0000000)
+#else
 #define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
 #endif
+#endif
 
 #ifndef	WIFSTOPPED
 #ifdef IBM6000
@@ -64,12 +82,21 @@
 #endif
 
 #ifndef	WEXITSTATUS
+#if defined (__MINGW32__)
+#define WEXITSTATUS(stat_val) ((stat_val) & 255)
+#else
 #define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
 #endif
+#endif
 
 #ifndef	WTERMSIG
+#if defined (__MINGW32__)
+extern enum gdb_signal windows_status_to_termsig (int stat_val);
+#define WTERMSIG(stat_val)    windows_status_to_termsig (stat_val)
+#else
 #define WTERMSIG(w)	((w) & 0177)
 #endif
+#endif
 
 #ifndef	WSTOPSIG
 #define WSTOPSIG	WEXITSTATUS
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index ae4e3d55b3..c90caeda6a 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -280,9 +280,9 @@ static const int *mappings;
    a segment register or not.  */
 static segment_register_p_ftype *segment_register_p;
 
-/* See windows_nat_target::resume to understand why this is commented
-   out.  */
-#if 0
+/* See windows_nat_target::resume to understand why xlate is not used
+   to translate a signal into an exception.  */
+
 /* This vector maps the target's idea of an exception (extracted
    from the DEBUG_EVENT structure) to GDB's idea.  */
 
@@ -302,7 +302,17 @@ static const struct xlate_exception xlate[] =
   {STATUS_FLOAT_DIVIDE_BY_ZERO, GDB_SIGNAL_FPE}
 };
 
-#endif /* 0 */
+/* Translate a windows exception inside STAT_VAL into a gdb_signal.
+   This should only be called if WIFSIGNALED (stat_val).  */
+
+enum gdb_signal
+windows_status_to_termsig (int stat_val)
+{
+  for (const xlate_exception &x : xlate)
+    if (x.them == (stat_val & ~0xC0000000))
+      return x.us;
+  return GDB_SIGNAL_UNKNOWN;
+}
 
 struct windows_nat_target final : public x86_nat_target<inf_child_target>
 {
-- 
2.20.1

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 5/4/19 5:17 PM, Philippe Waroquiers wrote:
> gdb/ChangeLog
> 2019-05-04  Eli Zaretskii  <eliz@gnu.org>
> 	    Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* common/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS,
> 	WTERMSIG): Define better versions for MinGW.
> 	* windows-nat.c (xlate): Uncomment the definition.
> 	(windows_status_to_termsig): New function.

windows-nat.c looks like the wrong place to put this.

windows-nat.c is only included in the build if building a native
debugger.  But, you need this functionality on every Windows-hosted build
of GDB, even cross debuggers.  So I think you're breaking the build on
the Windows-hosted, non-native-debugger case.

E.g., --host=mingw --target=arm-none-eabi.

The right place would be mingw-hdep.c.

I admit to being a bit confused about why we want to do this
translation for this feature while we don't do it for the exit code
of inferiors running under gdb, for example.  I mean, exit status
with 0xc0000000 set don't cause $_exitsignal to be set instead of
$_exitcode.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 27 May 2019 18:33:11 +0100
> 
> I admit to being a bit confused about why we want to do this
> translation for this feature while we don't do it for the exit code
> of inferiors running under gdb, for example.  I mean, exit status
> with 0xc0000000 set don't cause $_exitsignal to be set instead of
> $_exitcode.

We should probably do this everywhere where it matters whether the
inferior exited due to a fatal signal.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 5/27/19 7:37 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Mon, 27 May 2019 18:33:11 +0100
>>
>> I admit to being a bit confused about why we want to do this
>> translation for this feature while we don't do it for the exit code
>> of inferiors running under gdb, for example.  I mean, exit status
>> with 0xc0000000 set don't cause $_exitsignal to be set instead of
>> $_exitcode.
> 
> We should probably do this everywhere where it matters whether the
> inferior exited due to a fatal signal.

Wouldn't it better to leave that translation out of this patch series,
and do it everywhere it matters as a separate change, to avoid
creating inconsistencies?  Might be simpler for Philippe too, since
the current patch isn't OK as is.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 29 May 2019 13:37:56 +0100
> 
> > We should probably do this everywhere where it matters whether the
> > inferior exited due to a fatal signal.
> 
> Wouldn't it better to leave that translation out of this patch series,
> and do it everywhere it matters as a separate change, to avoid
> creating inconsistencies?  Might be simpler for Philippe too, since
> the current patch isn't OK as is.

It's up to you guys, I don't have a strong opinion about that.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Philippe Waroquiers]

On Wed, 2019-05-29 at 13:37 +0100, Pedro Alves wrote:
> On 5/27/19 7:37 PM, Eli Zaretskii wrote:
> > > From: Pedro Alves <palves@redhat.com>
> > > Date: Mon, 27 May 2019 18:33:11 +0100
> > > 
> > > I admit to being a bit confused about why we want to do this
> > > translation for this feature while we don't do it for the exit code
> > > of inferiors running under gdb, for example.  I mean, exit status
> > > with 0xc0000000 set don't cause $_exitsignal to be set instead of
> > > $_exitcode.
> > 
> > We should probably do this everywhere where it matters whether the
> > inferior exited due to a fatal signal.
> 
> Wouldn't it better to leave that translation out of this patch series,
> and do it everywhere it matters as a separate change, to avoid
> creating inconsistencies?  Might be simpler for Philippe too, since
> the current patch isn't OK as is.
Yes, that looks a better approach, in particular because I do not have
a platform where I can compile such builds ...

Philippe

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 27 May 2019 18:33:11 +0100

To recap, back in May Philippe added the 'pipe' command, and we had a
brief discussion regarding the use of WIFEXITED, WIFSIGNALED, and
other related macros from <sys/wait.h>, on MS-Windows.  It was decided
back then to leave for later the translation of exit codes returned by
'pipe' in MS-Windows build of GDB.

I've now started to look at this issue, with the intent to provide ext
status to signal conversion for the MS-Windows ports of GDB, and I
have a few questions regarding the details.

In that discussion, Pedro commented on Philippe's proposed patch
(https://sourceware.org/ml/gdb-patches/2019-05/msg00131.html) which
added the definitions for WIF* and WEXIT* macros to gdb_wait.h and
their use in widnows-nat.c.  The comments are in
https://sourceware.org/ml/gdb-patches/2019-05/msg00590.html, and go
like this:

> > 	* common/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS,
> > 	WTERMSIG): Define better versions for MinGW.
> > 	* windows-nat.c (xlate): Uncomment the definition.
> > 	(windows_status_to_termsig): New function.
> 
> windows-nat.c looks like the wrong place to put this.
> 
> windows-nat.c is only included in the build if building a native
> debugger.  But, you need this functionality on every Windows-hosted build
> of GDB, even cross debuggers.  So I think you're breaking the build on
> the Windows-hosted, non-native-debugger case.
> 
> E.g., --host=mingw --target=arm-none-eabi.
> 
> The right place would be mingw-hdep.c.

I'm okay with doing this in mingw-hdep.c, but I'm a bit confused by
this comment.  The encoding of the fatal exception in the exit status
of a program is a feature of the native MS-Windows processes.  Does
"running cross-debugger" mentioned above allude to running an
MS-Windows program?  If so, which GDB component (that is presumably
not windows-nat.c) is involved in running such cross-debugged programs
and for translating the debug status to the likes of
TARGET_WAITKIND_EXITED?  And how does the 'pipe' command support these
cross-debugging use cases (as it uses the 'popen' C library function,
which AFAIU runs natively)?

> I admit to being a bit confused about why we want to do this
> translation for this feature while we don't do it for the exit code
> of inferiors running under gdb, for example.  I mean, exit status
> with 0xc0000000 set don't cause $_exitsignal to be set instead of
> $_exitcode.

Yes, we should do this for exit code of inferiors as well.

Native MS-Windows debugging produces the TARGET_WAITKIND_* values in
windows-nat.c, so I think the conversion we are talking about will
have to be done there, perhaps _in_addition_to_ other places?  IOW,
the function that performs the conversion can be defined in
mingw-hdep.c, but it will have to be called from windows-nat.c at
least, right?  And I'm uncertain what other places will have to call
that conversion function for the use case of running a cross-debugger,
can someone please help me understand that?

TIA

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 12/17/19 4:59 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Mon, 27 May 2019 18:33:11 +0100
> 
> To recap, back in May Philippe added the 'pipe' command, and we had a
> brief discussion regarding the use of WIFEXITED, WIFSIGNALED, and
> other related macros from <sys/wait.h>, on MS-Windows.  It was decided
> back then to leave for later the translation of exit codes returned by
> 'pipe' in MS-Windows build of GDB.
> 
> I've now started to look at this issue, with the intent to provide ext
> status to signal conversion for the MS-Windows ports of GDB, and I
> have a few questions regarding the details.
> 
> In that discussion, Pedro commented on Philippe's proposed patch
> (https://sourceware.org/ml/gdb-patches/2019-05/msg00131.html) which
> added the definitions for WIF* and WEXIT* macros to gdb_wait.h and
> their use in widnows-nat.c.  The comments are in
> https://sourceware.org/ml/gdb-patches/2019-05/msg00590.html, and go
> like this:
> 
>>> 	* common/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS,
>>> 	WTERMSIG): Define better versions for MinGW.
>>> 	* windows-nat.c (xlate): Uncomment the definition.
>>> 	(windows_status_to_termsig): New function.
>>
>> windows-nat.c looks like the wrong place to put this.
>>
>> windows-nat.c is only included in the build if building a native
>> debugger.  But, you need this functionality on every Windows-hosted build
>> of GDB, even cross debuggers.  So I think you're breaking the build on
>> the Windows-hosted, non-native-debugger case.
>>
>> E.g., --host=mingw --target=arm-none-eabi.
>>
>> The right place would be mingw-hdep.c.
> 
> I'm okay with doing this in mingw-hdep.c, but I'm a bit confused by
> this comment.  The encoding of the fatal exception in the exit status
> of a program is a feature of the native MS-Windows processes.  Does
> "running cross-debugger" mentioned above allude to running an
> MS-Windows program?  

The issue pointed out was that by putting the windows_status_to_termsig
function in windows-nat.c, and then calling it from gdb's common code
(cli/cli-cmds.c, via WTERMSIG) would result in a build/link failure when
you try to build a cross debugger hosted on mingw, because such a gdb
build does not include the native Windows target support, i.e., does not
build/link the windows-nat.o object.  Putting said function in mingw-hdep.c
instead fixes that issue because that file is included as part of the build
in all kinds of mingw-hosted GDBs, either native or cross-debugger.

>> I admit to being a bit confused about why we want to do this
>> translation for this feature while we don't do it for the exit code
>> of inferiors running under gdb, for example.  I mean, exit status
>> with 0xc0000000 set don't cause $_exitsignal to be set instead of
>> $_exitcode.
> 
> Yes, we should do this for exit code of inferiors as well.
> 
> Native MS-Windows debugging produces the TARGET_WAITKIND_* values in
> windows-nat.c, so I think the conversion we are talking about will
> have to be done there, perhaps _in_addition_to_ other places?  IOW,
> the function that performs the conversion can be defined in
> mingw-hdep.c, but it will have to be called from windows-nat.c at
> least, right?  And I'm uncertain what other places will have to call
> that conversion function for the use case of running a cross-debugger,
> can someone please help me understand that?

You'll also want to call it in gdbserver's win32-low.c file, so that
you get the translation too when debugging against gdbserver.
This actually suggests putting the new function in some new
shared file in gdb/gdbsupport/, since gdb/mingw-hdep.c is gdb-only.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 17 Dec 2019 17:51:26 +0000
> 
> The issue pointed out was that by putting the windows_status_to_termsig
> function in windows-nat.c, and then calling it from gdb's common code
> (cli/cli-cmds.c, via WTERMSIG) would result in a build/link failure when
> you try to build a cross debugger hosted on mingw, because such a gdb
> build does not include the native Windows target support, i.e., does not
> build/link the windows-nat.o object.  Putting said function in mingw-hdep.c
> instead fixes that issue because that file is included as part of the build
> in all kinds of mingw-hosted GDBs, either native or cross-debugger.
> 
> >> I admit to being a bit confused about why we want to do this
> >> translation for this feature while we don't do it for the exit code
> >> of inferiors running under gdb, for example.  I mean, exit status
> >> with 0xc0000000 set don't cause $_exitsignal to be set instead of
> >> $_exitcode.
> > 
> > Yes, we should do this for exit code of inferiors as well.
> > 
> > Native MS-Windows debugging produces the TARGET_WAITKIND_* values in
> > windows-nat.c, so I think the conversion we are talking about will
> > have to be done there, perhaps _in_addition_to_ other places?  IOW,
> > the function that performs the conversion can be defined in
> > mingw-hdep.c, but it will have to be called from windows-nat.c at
> > least, right?  And I'm uncertain what other places will have to call
> > that conversion function for the use case of running a cross-debugger,
> > can someone please help me understand that?
> 
> You'll also want to call it in gdbserver's win32-low.c file, so that
> you get the translation too when debugging against gdbserver.
> This actually suggests putting the new function in some new
> shared file in gdb/gdbsupport/, since gdb/mingw-hdep.c is gdb-only.

A new file for just one function sounds too much to me.  Is it OK to
define an inline function in gdb_wait.h, as in the prototype change
below?  If this way is accepted, I will post a fully formatted patch.

Thanks.

--- gdb/gdbsupport/gdb_wait.h~0	2019-09-21 00:58:17.000000000 +0300
+++ gdb/gdbsupport/gdb_wait.h	2019-12-18 17:59:28.434097000 +0200
@@ -40,18 +40,84 @@
    NOTE exception for GNU/Linux below).  We also fail to declare
    wait() and waitpid().  */
 
+/* For MINGW, the underlying idea is that when a Windows program is
+   terminated by a fatal exception, its exit code is the value of that
+   exception, as defined by the various EXCEPTION_* symbols in the
+   Windows API headers.
+
+   The translation below is not perfect, because a program could
+   legitimately exit normally with a status whose value happens to
+   have the high bits set, but that's extremely rare, to say the
+   least, and it is deemed such a negligibly small probability of
+   false positives is justified by the utility of reporting the
+   terminating signal in the "normal" cases.  */
+
+#ifdef __MINGW32__
+
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>		/* for EXCEPTION_* constants */
+# include "gdb/signals.h"	/* for enum gdb_signal */
+
+struct xlate_status
+{
+  DWORD status;		/* exit status (actually, fatal exception code) */
+  enum gdb_signal sig;	/* corresponding GDB signal value */
+};
+
+static inline enum gdb_signal
+windows_status_to_termsig (DWORD status)
+{
+  static const xlate_status status_xlate_tbl[] =
+    {
+     {EXCEPTION_ACCESS_VIOLATION, 	  GDB_SIGNAL_SEGV},
+     {EXCEPTION_IN_PAGE_ERROR,		  GDB_SIGNAL_SEGV},
+     {EXCEPTION_INVALID_HANDLE, 	  GDB_SIGNAL_SEGV},
+     {EXCEPTION_ILLEGAL_INSTRUCTION, 	  GDB_SIGNAL_ILL},
+     {EXCEPTION_NONCONTINUABLE_EXCEPTION, GDB_SIGNAL_ILL},
+     {EXCEPTION_ARRAY_BOUNDS_EXCEEDED, 	  GDB_SIGNAL_SEGV},
+     {EXCEPTION_FLT_DENORMAL_OPERAND, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_INEXACT_RESULT, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_INVALID_OPERATION, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_OVERFLOW, 		  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_STACK_CHECK, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_UNDERFLOW, 		  GDB_SIGNAL_FPE},
+     {EXCEPTION_INT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_INT_OVERFLOW, 		  GDB_SIGNAL_FPE},
+     {EXCEPTION_PRIV_INSTRUCTION, 	  GDB_SIGNAL_ILL},
+     {EXCEPTION_STACK_OVERFLOW, 	  GDB_SIGNAL_SEGV},
+     {CONTROL_C_EXIT, 			  GDB_SIGNAL_TERM}
+    };
+
+  for (const xlate_status &x : status_xlate_tbl)
+    if (x.status == status)
+      return x.sig;
+
+  return GDB_SIGNAL_UNKNOWN;
+}
+
+#endif	/* __MINGW32__ */
+
 #ifndef	WIFEXITED
-#define WIFEXITED(w)	(((w)&0377) == 0)
+# ifdef __MINGW32__
+#  define WIFEXITED(w)	(((w) & 0xC0000000) == 0)
+# else
+#  define WIFEXITED(w)	(((w)&0377) == 0)
+# endif
 #endif
 
 #ifndef	WIFSIGNALED
-#define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
+# ifdef __MINGW32__
+#  define WIFSIGNALED(w)	(((w) & 0xC0000000) == 0xC0000000)
+# else
+#  define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
+# endif
 #endif
 
 #ifndef	WIFSTOPPED
 #ifdef IBM6000
 
-/* Unfortunately, the above comment (about being compatible in all Unix 
+/* Unfortunately, the above comment (about being compatible in all Unix
    systems) is not quite correct for AIX, sigh.  And AIX 3.2 can generate
    status words like 0x57c (sigtrap received after load), and gdb would
    choke on it.  */
@@ -64,11 +130,19 @@
 #endif
 
 #ifndef	WEXITSTATUS
-#define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
+# ifdef __MINGW32__
+#  define WEXITSTATUS(w)	((w) & ~0xC0000000)
+# else
+#  define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
+# endif
 #endif
 
 #ifndef	WTERMSIG
-#define WTERMSIG(w)	((w) & 0177)
+# ifdef __MINGW32__
+#  define WTERMSIG(w)	windows_status_to_termsig (w)
+# else
+#  define WTERMSIG(w)	((w) & 0177)
+# endif
 #endif
 
 #ifndef	WSTOPSIG
--- gdb/windows-nat.c~0	2019-12-11 22:24:51.000000000 +0200
+++ gdb/windows-nat.c	2019-12-18 18:21:00.264558400 +0200
@@ -68,6 +68,7 @@
 #include "inf-child.h"
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_wait.h"
 
 #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
 #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
@@ -1620,8 +1621,17 @@ get_windows_debug_event (struct target_o
 	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
 					 current_event.dwThreadId),
 				 0, true /* main_thread_p */);
-	  ourstatus->kind = TARGET_WAITKIND_EXITED;
-	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
+	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
+	  if (WIFEXITED (exit_status))
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_EXITED;
+	      ourstatus->value.integer = WEXITSTATUS (exit_status);
+	    }
+	  else
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+	      ourstatus->value.sig = WTERMSIG (exit_status);
+	    }
 	  thread_id = current_event.dwThreadId;
 	}
       break;
--- gdb/windows-tdep.c~0	2019-09-21 00:58:17.000000000 +0300
+++ gdb/windows-tdep.c	2019-12-18 17:49:52.360580700 +0200
@@ -35,6 +35,8 @@
 #include "solib-target.h"
 #include "gdbcore.h"
 
+#include <signal.h>
+
 struct cmd_list_element *info_w32_cmdlist;
 
 typedef struct thread_information_block_32
@@ -461,6 +463,29 @@ init_w32_command_list (void)
     }
 }
 
+static int
+windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
+{
+  switch (signal)
+    {
+    case GDB_SIGNAL_0:
+      return 0;
+    case GDB_SIGNAL_INT:
+      return SIGINT;
+    case GDB_SIGNAL_ILL:
+      return SIGILL;
+    case GDB_SIGNAL_ABRT:
+      return SIGABRT;
+    case GDB_SIGNAL_FPE:
+      return SIGFPE;
+    case GDB_SIGNAL_SEGV:
+      return SIGSEGV;
+    case GDB_SIGNAL_TERM:
+      return SIGTERM;
+    }
+  return -1;
+}
+
 /* To be called from the various GDB_OSABI_CYGWIN handlers for the
    various Windows architectures and machine types.  */
 
@@ -477,6 +502,8 @@ windows_init_abi (struct gdbarch_info in
   set_gdbarch_iterate_over_objfiles_in_search_order
     (gdbarch, windows_iterate_over_objfiles_in_search_order);
 
+  set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target);
+
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
 
--- gdb/gdbserver/win32-low.c~0	2019-11-19 03:10:41.000000000 +0200
+++ gdb/gdbserver/win32-low.c	2019-12-18 17:51:32.098324200 +0200
@@ -34,6 +34,7 @@
 #include <process.h>
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/common-inferior.h"
+#include "gdbsupport/gdb_wait.h"
 
 #ifndef USE_WIN32API
 #include <sys/cygwin.h>
@@ -1511,8 +1512,19 @@ get_child_debug_event (struct target_wai
 		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
-      ourstatus->kind = TARGET_WAITKIND_EXITED;
-      ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
+      {
+	DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
+	if (WIFEXITED (exit_status))
+	  {
+	    ourstatus->kind = TARGET_WAITKIND_EXITED;
+	    ourstatus->value.integer = WEXITSTATUS (exit_status);
+	  }
+	else
+	  {
+	    ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+	    ourstatus->value.sig = WTERMSIG (exit_status);
+	  }
+      }
       child_continue (DBG_CONTINUE, -1);
       CloseHandle (current_process_handle);
       current_process_handle = NULL;
@@ -1607,6 +1619,7 @@ win32_wait (ptid_t ptid, struct target_w
 	  win32_clear_inferiors ();
 	  return ptid_t (current_event.dwProcessId);
 	case TARGET_WAITKIND_STOPPED:
+	case TARGET_WAITKIND_SIGNALLED:
 	case TARGET_WAITKIND_LOADED:
 	  OUTMSG2 (("Child Stopped with signal = %d \n",
 		    ourstatus->value.sig));

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 12/18/19 5:07 PM, Eli Zaretskii wrote:
>> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 17 Dec 2019 17:51:26 +0000
>>
>> The issue pointed out was that by putting the windows_status_to_termsig
>> function in windows-nat.c, and then calling it from gdb's common code
>> (cli/cli-cmds.c, via WTERMSIG) would result in a build/link failure when
>> you try to build a cross debugger hosted on mingw, because such a gdb
>> build does not include the native Windows target support, i.e., does not
>> build/link the windows-nat.o object.  Putting said function in mingw-hdep.c
>> instead fixes that issue because that file is included as part of the build
>> in all kinds of mingw-hosted GDBs, either native or cross-debugger.
>>
>>>> I admit to being a bit confused about why we want to do this
>>>> translation for this feature while we don't do it for the exit code
>>>> of inferiors running under gdb, for example.  I mean, exit status
>>>> with 0xc0000000 set don't cause $_exitsignal to be set instead of
>>>> $_exitcode.
>>>
>>> Yes, we should do this for exit code of inferiors as well.
>>>
>>> Native MS-Windows debugging produces the TARGET_WAITKIND_* values in
>>> windows-nat.c, so I think the conversion we are talking about will
>>> have to be done there, perhaps _in_addition_to_ other places?  IOW,
>>> the function that performs the conversion can be defined in
>>> mingw-hdep.c, but it will have to be called from windows-nat.c at
>>> least, right?  And I'm uncertain what other places will have to call
>>> that conversion function for the use case of running a cross-debugger,
>>> can someone please help me understand that?
>>
>> You'll also want to call it in gdbserver's win32-low.c file, so that
>> you get the translation too when debugging against gdbserver.
>> This actually suggests putting the new function in some new
>> shared file in gdb/gdbsupport/, since gdb/mingw-hdep.c is gdb-only.
> 
> A new file for just one function sounds too much to me.  Is it OK to
> define an inline function in gdb_wait.h, as in the prototype change
> below?  If this way is accepted, I will post a fully formatted patch.

I don't think it's too much.  As a static inline function means that
you end up with multiple versions of the function, including
the mapping array, in the gdb binary.  And also make gdb_wait.h
expose <windows.h>.  You could add a new gdbsupport/gdb_wait.c
file, with the function wrapped in #ifdef __MINGW32__ to keep it simple
and avoid host/target checks in the configure.ac files.

> 
> Thanks.
> 
> --- gdb/gdbsupport/gdb_wait.h~0	2019-09-21 00:58:17.000000000 +0300
> +++ gdb/gdbsupport/gdb_wait.h	2019-12-18 17:59:28.434097000 +0200
> @@ -40,18 +40,84 @@
>     NOTE exception for GNU/Linux below).  We also fail to declare
>     wait() and waitpid().  */
>  
> +/* For MINGW, the underlying idea is that when a Windows program is
> +   terminated by a fatal exception, its exit code is the value of that
> +   exception, as defined by the various EXCEPTION_* symbols in the
> +   Windows API headers.
> +
> +   The translation below is not perfect, because a program could
> +   legitimately exit normally with a status whose value happens to
> +   have the high bits set, but that's extremely rare, to say the
> +   least, and it is deemed such a negligibly small probability of
> +   false positives is justified by the utility of reporting the
> +   terminating signal in the "normal" cases.  */
> +
> +#ifdef __MINGW32__
> +
> +# define WIN32_LEAN_AND_MEAN
> +# include <windows.h>		/* for EXCEPTION_* constants */
> +# include "gdb/signals.h"	/* for enum gdb_signal */
> +
> +struct xlate_status
> +{
> +  DWORD status;		/* exit status (actually, fatal exception code) */
> +  enum gdb_signal sig;	/* corresponding GDB signal value */
> +};
> +
> +static inline enum gdb_signal
> +windows_status_to_termsig (DWORD status)
> +{
> +  static const xlate_status status_xlate_tbl[] =
> +    {
> +     {EXCEPTION_ACCESS_VIOLATION, 	  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_IN_PAGE_ERROR,		  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_INVALID_HANDLE, 	  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_ILLEGAL_INSTRUCTION, 	  GDB_SIGNAL_ILL},
> +     {EXCEPTION_NONCONTINUABLE_EXCEPTION, GDB_SIGNAL_ILL},
> +     {EXCEPTION_ARRAY_BOUNDS_EXCEEDED, 	  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_FLT_DENORMAL_OPERAND, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_INEXACT_RESULT, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_INVALID_OPERATION, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_OVERFLOW, 		  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_STACK_CHECK, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_UNDERFLOW, 		  GDB_SIGNAL_FPE},
> +     {EXCEPTION_INT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_INT_OVERFLOW, 		  GDB_SIGNAL_FPE},
> +     {EXCEPTION_PRIV_INSTRUCTION, 	  GDB_SIGNAL_ILL},
> +     {EXCEPTION_STACK_OVERFLOW, 	  GDB_SIGNAL_SEGV},
> +     {CONTROL_C_EXIT, 			  GDB_SIGNAL_TERM}
> +    };
> +
> +  for (const xlate_status &x : status_xlate_tbl)
> +    if (x.status == status)
> +      return x.sig;
> +
> +  return GDB_SIGNAL_UNKNOWN;
> +}
> +
> +#endif	/* __MINGW32__ */
> +
>  #ifndef	WIFEXITED
> -#define WIFEXITED(w)	(((w)&0377) == 0)
> +# ifdef __MINGW32__
> +#  define WIFEXITED(w)	(((w) & 0xC0000000) == 0)
> +# else
> +#  define WIFEXITED(w)	(((w)&0377) == 0)
> +# endif
>  #endif
>  
>  #ifndef	WIFSIGNALED
> -#define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
> +# ifdef __MINGW32__
> +#  define WIFSIGNALED(w)	(((w) & 0xC0000000) == 0xC0000000)
> +# else
> +#  define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
> +# endif
>  #endif
>  
>  #ifndef	WIFSTOPPED
>  #ifdef IBM6000
>  
> -/* Unfortunately, the above comment (about being compatible in all Unix 
> +/* Unfortunately, the above comment (about being compatible in all Unix
>     systems) is not quite correct for AIX, sigh.  And AIX 3.2 can generate
>     status words like 0x57c (sigtrap received after load), and gdb would
>     choke on it.  */
> @@ -64,11 +130,19 @@
>  #endif
>  
>  #ifndef	WEXITSTATUS
> -#define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
> +# ifdef __MINGW32__
> +#  define WEXITSTATUS(w)	((w) & ~0xC0000000)
> +# else
> +#  define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
> +# endif
>  #endif
>  
>  #ifndef	WTERMSIG
> -#define WTERMSIG(w)	((w) & 0177)
> +# ifdef __MINGW32__
> +#  define WTERMSIG(w)	windows_status_to_termsig (w)
> +# else
> +#  define WTERMSIG(w)	((w) & 0177)
> +# endif
>  #endif
>  
>  #ifndef	WSTOPSIG
> --- gdb/windows-nat.c~0	2019-12-11 22:24:51.000000000 +0200
> +++ gdb/windows-nat.c	2019-12-18 18:21:00.264558400 +0200
> @@ -68,6 +68,7 @@
>  #include "inf-child.h"
>  #include "gdbsupport/gdb_tilde_expand.h"
>  #include "gdbsupport/pathstuff.h"
> +#include "gdbsupport/gdb_wait.h"
>  
>  #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
>  #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
> @@ -1620,8 +1621,17 @@ get_windows_debug_event (struct target_o
>  	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
>  					 current_event.dwThreadId),
>  				 0, true /* main_thread_p */);
> -	  ourstatus->kind = TARGET_WAITKIND_EXITED;
> -	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
> +	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
> +	  if (WIFEXITED (exit_status))
> +	    {
> +	      ourstatus->kind = TARGET_WAITKIND_EXITED;
> +	      ourstatus->value.integer = WEXITSTATUS (exit_status);
> +	    }
> +	  else
> +	    {
> +	      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
> +	      ourstatus->value.sig = WTERMSIG (exit_status);
> +	    }
>  	  thread_id = current_event.dwThreadId;
>  	}
>        break;
> --- gdb/windows-tdep.c~0	2019-09-21 00:58:17.000000000 +0300
> +++ gdb/windows-tdep.c	2019-12-18 17:49:52.360580700 +0200
> @@ -35,6 +35,8 @@
>  #include "solib-target.h"
>  #include "gdbcore.h"
>  
> +#include <signal.h>
> +
>  
> +static int
> +windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
> +{

We should not rely on signal.h constants for this hook.  See gdbarch.sh:

 # Signal translation: translate the GDB's internal signal number into
 # the inferior's signal (target's) representation.  The implementation
 # of this method must be host independent.  IOW, don't rely on symbols
 # of the NAT_FILE header (the nm-*.h files), the host <signal.h>
 # header, or similar headers.
 # Return the target signal number if found, or -1 if the GDB internal
 # signal number is invalid.
 M;int;gdb_signal_to_target;enum gdb_signal signal;signal

Say you're debugging against a mingw gdbserver from a Linux host.
If you rely on <signal.h> constants here, this function is going to
return the Linux (or whatever the host) numbers instead of the
Windows/mingw numbers.  For Linux, we define the LINUX_SIGxxx numbers
in linux-tdep.c, around line 125.  The patch should add a similar
enum.

Some overall comments:

With this change, the user no longer has access to the original
$_exitcode, for the cases that match one of the known exceptions.
I don't know whether anyone is relying on those, though I wouldn't
be surprised if so.  I assume you've pondered about this and consider 
that the change is still worth it anyhow.  This should at least be
documented.  I wonder whether we should provide both the exit
code in $_exitcode and the translated signal number in $_exitsignal,
though that would require more changes.

Related, when windows_status_to_termsig doesn't recognize the
exception number, we end up with GDB_SIGNAL_UNKNOWN, and then
$_exitsignal == -1.  I.e., we lose information in that case.  Again,
something to ponder about that information loss is OK, or whether
we should do something about it.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 18 Dec 2019 17:42:28 +0000
> 
> > A new file for just one function sounds too much to me.  Is it OK to
> > define an inline function in gdb_wait.h, as in the prototype change
> > below?  If this way is accepted, I will post a fully formatted patch.
> 
> I don't think it's too much.  As a static inline function means that
> you end up with multiple versions of the function, including
> the mapping array, in the gdb binary.  And also make gdb_wait.h
> expose <windows.h>.  You could add a new gdbsupport/gdb_wait.c
> file, with the function wrapped in #ifdef __MINGW32__ to keep it simple
> and avoid host/target checks in the configure.ac files.

Sorry, I don't understand what host/target checks might be needed in
configure.ac, can you explain?

> We should not rely on signal.h constants for this hook.  See gdbarch.sh:
> 
>  # Signal translation: translate the GDB's internal signal number into
>  # the inferior's signal (target's) representation.  The implementation
>  # of this method must be host independent.  IOW, don't rely on symbols
>  # of the NAT_FILE header (the nm-*.h files), the host <signal.h>
>  # header, or similar headers.
>  # Return the target signal number if found, or -1 if the GDB internal
>  # signal number is invalid.
>  M;int;gdb_signal_to_target;enum gdb_signal signal;signal
> 
> Say you're debugging against a mingw gdbserver from a Linux host.
> If you rely on <signal.h> constants here, this function is going to
> return the Linux (or whatever the host) numbers instead of the
> Windows/mingw numbers.  For Linux, we define the LINUX_SIGxxx numbers
> in linux-tdep.c, around line 125.  The patch should add a similar
> enum.

So we need to have 2 different sets of explicit signal numbers, one
for MinGW64, the other for mingw.org's MinGW (no, they are not
identical)?  And perhaps one more for Cygwin?  Or should we just
support the signals common to all 3 environments?

And what do we do when the signal numbers in the system's signal.h
header change in some future version of MinGW/Cygwin?  This sounds
like a very fragile arrangement, at least for non-Posix systems where
signals are emulated and aren't part of the kernel definitions set in
stone.  I'm okay with doing a bunch of defines, but it seems to me a
maintenance headache in the long run, FWIW.

> With this change, the user no longer has access to the original
> $_exitcode, for the cases that match one of the known exceptions.
> I don't know whether anyone is relying on those, though I wouldn't
> be surprised if so.  I assume you've pondered about this and consider 
> that the change is still worth it anyhow.  This should at least be
> documented.  I wonder whether we should provide both the exit
> code in $_exitcode and the translated signal number in $_exitsignal,
> though that would require more changes.

I think this is a micro-optimization that is way in the area of the
diminishing returns.  I've never seen a program to return an exit
status with the high 2 bits set.  I'd definitely not recommend to
tweak the current assumption in the GDB code that if a program exited
due to a signal, its exit code is irrelevant, based on such a
theoretical possibility.  In most cases, the fact that the inferior
got a fatal exception will be noted before it exits anyway.

> Related, when windows_status_to_termsig doesn't recognize the
> exception number, we end up with GDB_SIGNAL_UNKNOWN, and then
> $_exitsignal == -1.  I.e., we lose information in that case.  Again,
> something to ponder about that information loss is OK, or whether
> we should do something about it.

I don't think we should do anything about it, it's highly theoretical
situation I've never seen in real life.  And we already lose
information in windows-nat.c, when we return GDB_SIGNAL_UNKNOWN for
any exception we don't recognize explicitly.  In any case, the way
this stuff currently works, I see no simple way of returning an
arbitrary value of a signal.

Maybe we should abandon the idea of doing this in windows-nat.c and
win32-low.c, and only translate the exit code in cli-cmds.c for the
'pipe' command?  It's much more important in that case (since we don't
intercept the signals as we do from the inferior), and most of those
complications don't apply there.  There's also no backward
compatibility problem, since 'pipe' is a new feature in GDB 9, with 2
new convenience variables to hold the exit status and the signal
value.  WDYT?

Thanks.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

No further comments, so I made additional changes to take care of
Pedro's review comments, and the result is below.

OK to commit?

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index acf9106..838eafd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,26 @@
+2019-12-25  Eli Zaretskii  <eliz@gnu.org>
+
+	* windows-tdep.c: New enumeration of WINDOWS_SIG* signals.
+	(windows_gdb_signal_to_target): New function, uses the above
+	enumeration to convert GDB internal signal codes to equivalent
+	Windows codes.
+	(windows_init_abi): Call set_gdbarch_gdb_signal_to_target.
+
+	* windows-nat.c (get_windows_debug_event): Extract the fatal
+	exception from the exit status and convert to the equivalent Posix
+	signal number.
+
+	* gdbsupport/gdb_wait.c: New file, implements
+	windows_status_to_termsig.
+
+	* gdbsupport/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS)
+	(WTERMSIG) [__MINGW32__]: Separate definitions for MinGW.
+
+	* cli/cli-cmds.c (exit_status_set_internal_vars): Account for the
+	possibility that WTERMSIG returns GDB_SIGNAL_UNKNOWN.
+
+	* Makefile.in (COMMON_SFILES): Add gdbsupport/gdb_wait.c.
+
 2019-12-21  George Barrett  <bob@bob131.so>
 
 	* solib-svr4.c (svr4_handle_solib_event): Add fallback link
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index fa5c820..34d0acf 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -986,6 +986,7 @@ COMMON_SFILES = \
 	gdbsupport/gdb-dlfcn.c \
 	gdbsupport/gdb_tilde_expand.c \
 	gdbsupport/gdb_vecs.c \
+	gdbsupport/gdb_wait.c \
 	gdbsupport/netstuff.c \
 	gdbsupport/new-op.c \
 	gdbsupport/pathstuff.c \
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 681d53c..0d1584e 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -798,10 +798,16 @@ exit_status_set_internal_vars (int exit_status)
 
   clear_internalvar (var_code);
   clear_internalvar (var_signal);
-  if (WIFEXITED (exit_status))
-    set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
-  else if (WIFSIGNALED (exit_status))
+  enum gdb_signal exit_signal
+    = WIFSIGNALED (exit_status) ? WTERMSIG (exit_status) : GDB_SIGNAL_UNKNOWN;
+  /* The GDB_SIGNAL_UNKNOWN condition can happen on MinGW, if we don't
+     recognize the fatal exception code encoded in the exit status;
+     see gdb_wait.c.  We don't want to lose information in the exit
+     status in that case.  */
+  if (exit_signal != GDB_SIGNAL_UNKNOWN)
     set_internalvar_integer (var_signal, WTERMSIG (exit_status));
+  else if (!WIFSIGNALED (exit_status))
+    set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
   else
     warning (_("unexpected shell command exit status %d"), exit_status);
 }

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 10d5c95..12e50b6 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -68,6 +68,7 @@
 #include "inf-child.h"
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_wait.h"
 
 #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
 #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
@@ -1620,8 +1621,22 @@ get_windows_debug_event (struct target_ops *ops,
 	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
 					 current_event.dwThreadId),
 				 0, true /* main_thread_p */);
-	  ourstatus->kind = TARGET_WAITKIND_EXITED;
-	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
+	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
+	  /* If the exit status looks like a fatal exception, but we
+	     don't recognize the exception's code, make the original
+	     exit status value available, to avoid losing information.  */
+	  enum gdb_signal exit_signal = WIFSIGNALED (exit_status)
+	    ? WTERMSIG (exit_status) : GDB_SIGNAL_UNKNOWN;
+	  if (exit_signal == GDB_SIGNAL_UNKNOWN)
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_EXITED;
+	      ourstatus->value.integer = exit_status;
+	    }
+	  else
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+	      ourstatus->value.sig = exit_signal;
+	    }
 	  thread_id = current_event.dwThreadId;
 	}
       break;
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index bb69a79..c2cecce 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -35,6 +35,55 @@
 #include "solib-target.h"
 #include "gdbcore.h"
 
+/* Windows signal numbers differ between MinGW flavors and between
+   those and Cygwin.  The below enumeration was gleaned from the
+   respective headers; the ones marked with MinGW64/Cygwin are defined
+   only by MinGW64 and Cygwin, not by mingw.org's MinGW.  */
+
+enum
+  {
+   WINDOWS_SIGHUP = 1,	/* MinGW64/Cygwin */
+   WINDOWS_SIGINT = 2,
+   WINDOWS_SIGQUIT = 3,	/* MinGW64/Cygwin */
+   WINDOWS_SIGILL = 4,
+   WINDOWS_SIGTRAP = 5,	/* MinGW64/Cygwin */
+#ifdef __CYGWIN__
+   WINDOWS_SGABRT = 6,
+#else
+   WINDOWS_SIGIOT = 6,	/* MinGW64 */
+#endif
+   WINDOWS_SIGEMT = 7,	/* MinGW64/Cygwin */
+   WINDOWS_SIGFPE = 8,
+   WINDOWS_SIGKILL = 9,	/* MinGW64/Cygwin */
+   WINDOWS_SIGBUS = 10,	/* MinGW64/Cygwin */
+   WINDOWS_SIGSEGV = 11,
+   WINDOWS_SIGSYS = 12,	/* MinGW64/Cygwin */
+   WINDOWS_SIGPIPE = 13,/* MinGW64/Cygwin */
+   WINDOWS_SIGALRM = 14,/* MinGW64/Cygwin */
+   WINDOWS_SIGTERM = 15,
+#ifdef __CYGWIN__
+   WINDOWS_SIGURG = 16,
+   WINDOWS_SIGSTOP = 17,
+   WINDOWS_SIGTSTP = 18,
+   WINDOWS_SIGCONT = 19,
+   WINDOWS_SIGCHLD = 20,
+   WINDOWS_SIGTTIN = 21,
+   WINDOWS_SIGTTOU = 22,
+   WINDOWS_SIGIO = 23,
+   WINDOWS_SIGXCPU = 24,
+   WINDOWS_SIGXFSZ = 25,
+   WINDOWS_SIGVTALRM = 26,
+   WINDOWS_SIGPROF = 27,
+   WINDOWS_SIGWINCH = 28,
+   WINDOWS_SIGLOST = 29,
+   WINDOWS_SIGUSR1 = 30,
+   WINDOWS_SIGUSR2 = 31
+#else
+   WINDOWS_SIGBREAK = 21,
+   WINDOWS_SIGABRT = 22
+#endif
+  };
+
 struct cmd_list_element *info_w32_cmdlist;
 
 typedef struct thread_information_block_32
@@ -461,6 +510,81 @@ init_w32_command_list (void)
     }
 }
 
+static int
+windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
+{
+  switch (signal)
+    {
+    case GDB_SIGNAL_0:
+      return 0;
+    case GDB_SIGNAL_HUP:
+      return WINDOWS_SIGHUP;
+    case GDB_SIGNAL_INT:
+      return WINDOWS_SIGINT;
+    case GDB_SIGNAL_QUIT:
+      return WINDOWS_SIGQUIT;
+    case GDB_SIGNAL_ILL:
+      return WINDOWS_SIGILL;
+    case GDB_SIGNAL_TRAP:
+      return WINDOWS_SIGTRAP;
+    case GDB_SIGNAL_ABRT:
+      return WINDOWS_SIGABRT;
+    case GDB_SIGNAL_EMT:
+      return WINDOWS_SIGEMT;
+    case GDB_SIGNAL_FPE:
+      return WINDOWS_SIGFPE;
+    case GDB_SIGNAL_KILL:
+      return WINDOWS_SIGKILL;
+    case GDB_SIGNAL_BUS:
+      return WINDOWS_SIGBUS;
+    case GDB_SIGNAL_SEGV:
+      return WINDOWS_SIGSEGV;
+    case GDB_SIGNAL_SYS:
+      return WINDOWS_SIGSYS;
+    case GDB_SIGNAL_PIPE:
+      return WINDOWS_SIGPIPE;
+    case GDB_SIGNAL_ALRM:
+      return WINDOWS_SIGALRM;
+    case GDB_SIGNAL_TERM:
+      return WINDOWS_SIGTERM;
+#ifdef __CYGWIN__
+    case GDB_SIGNAL_URG:
+      return WINDOWS_SIGURG;
+    case GDB_SIGNAL_STOP:
+      return WINDOWS_SIGSTOP;
+    case GDB_SIGNAL_TSTP:
+      return WINDOWS_SIGTSTP;
+    case GDB_SIGNAL_CONT:
+      return WINDOWS_SIGCONT;
+    case GDB_SIGNAL_CHLD:
+      return WINDOWS_SIGCHLD;
+    case GDB_SIGNAL_TTIN:
+      return WINDOWS_SIGTTIN;
+    case GDB_SIGNAL_TTOU:
+      return WINDOWS_SIGTTOU;
+    case GDB_SIGNAL_IO:
+      return WINDOWS_SIGIO;
+    case GDB_SIGNAL_XCPU:
+      return WINDOWS_SIGXCPU;
+    case GDB_SIGNAL_XFSZ:
+      return WINDOWS_SIGXFSZ;
+    case GDB_SIGNAL_VTALRM:
+      return WINDOWS_SIGVTALRM;
+    case GDB_SIGNAL_PROF:
+      return WINDOWS_SIGPROF;
+    case GDB_SIGNAL_WINCH:
+      return WINDOWS_SIGWINCH;
+    case GDB_SIGNAL_PWR:
+      return WINDOWS_SIGLOST;
+    case GDB_SIGNAL_USR1:
+      return WINDOWS_SIGUSR1;
+    case GDB_SIGNAL_USR2:
+      return WINDOWS_SIGUSR2;
+#endif	/* __CYGWIN__ */
+    }
+  return -1;
+}
+
 /* To be called from the various GDB_OSABI_CYGWIN handlers for the
    various Windows architectures and machine types.  */
 
@@ -477,6 +601,8 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_iterate_over_objfiles_in_search_order
     (gdbarch, windows_iterate_over_objfiles_in_search_order);
 
+  set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target);
+
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
 

diff --git a/gdb/gdbsupport/gdb_wait.h b/gdb/gdbsupport/gdb_wait.h
index b3b752c..e8597f9 100644
--- a/gdb/gdbsupport/gdb_wait.h
+++ b/gdb/gdbsupport/gdb_wait.h
@@ -38,20 +38,33 @@
    in POSIX.1.  We fail to define WNOHANG and WUNTRACED, which POSIX.1
    <sys/wait.h> defines, since our code does not use waitpid() (but
    NOTE exception for GNU/Linux below).  We also fail to declare
-   wait() and waitpid().  */
+   wait() and waitpid().
+
+   For MinGW, we use the fact that when a Windows program is
+   terminated by a fatal exception, its exit code is the value of that
+   exception, as defined by the various EXCEPTION_* symbols in the
+   Windows API headers.  See also gdb_wait.c.  */
 
 #ifndef	WIFEXITED
-#define WIFEXITED(w)	(((w)&0377) == 0)
+# ifdef __MINGW32__
+#  define WIFEXITED(w)	(((w) & 0xC0000000) == 0)
+# else
+#  define WIFEXITED(w)	(((w)&0377) == 0)
+# endif
 #endif
 
 #ifndef	WIFSIGNALED
-#define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
+# ifdef __MINGW32__
+#  define WIFSIGNALED(w)	(((w) & 0xC0000000) == 0xC0000000)
+# else
+#  define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
+# endif
 #endif
 
 #ifndef	WIFSTOPPED
 #ifdef IBM6000
 
-/* Unfortunately, the above comment (about being compatible in all Unix 
+/* Unfortunately, the above comment (about being compatible in all Unix
    systems) is not quite correct for AIX, sigh.  And AIX 3.2 can generate
    status words like 0x57c (sigtrap received after load), and gdb would
    choke on it.  */
@@ -64,11 +77,20 @@
 #endif
 
 #ifndef	WEXITSTATUS
-#define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
+# ifdef __MINGW32__
+#  define WEXITSTATUS(w)	((w) & ~0xC0000000)
+# else
+#  define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
+# endif
 #endif
 
 #ifndef	WTERMSIG
-#define WTERMSIG(w)	((w) & 0177)
+# ifdef __MINGW32__
+extern enum gdb_signal windows_status_to_termsig (unsigned long);
+#  define WTERMSIG(w)	windows_status_to_termsig (w)
+# else
+#  define WTERMSIG(w)	((w) & 0177)
+# endif
 #endif
 
 #ifndef	WSTOPSIG

 
diff --git a/gdb/gdbsupport/gdb_wait.c b/gdb/gdbsupport/gdb_wait.c
new file mode 100644
index 0000000..7096152
--- /dev/null
+++ b/gdb/gdbsupport/gdb_wait.c
@@ -0,0 +1,80 @@
+/* Support code for standard wait macros in gdb_wait.h.
+
+   Copyright (C) 2019 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 "common-defs.h"
+
+#ifdef __MINGW32__
+
+/* The underlying idea is that when a Windows program is terminated by
+   a fatal exception, its exit code is the value of that exception, as
+   defined by the various EXCEPTION_* symbols in the Windows API
+   headers.  We thus emulate WTERMSIG etc. by translating the fatal
+   exception codes to more-or-less equivalent Posix signals.
+
+   The translation below is not perfect, because a program could
+   legitimately exit normally with a status whose value happens to
+   have the high bits set, but that's extremely rare, to say the
+   least, and it is deemed such a negligibly small probability of
+   false positives is justified by the utility of reporting the
+   terminating signal in the "normal" cases.  */
+
+# include "gdb/signals.h"	/* for enum gdb_signal */
+
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>		/* for EXCEPTION_* constants */
+
+struct xlate_status
+{
+  DWORD status;		/* exit status (actually, fatal exception code) */
+  enum gdb_signal sig;	/* corresponding GDB signal value */
+};
+
+enum gdb_signal
+windows_status_to_termsig (unsigned long status)
+{
+  static const xlate_status status_xlate_tbl[] =
+    {
+     {EXCEPTION_ACCESS_VIOLATION, 	  GDB_SIGNAL_SEGV},
+     {EXCEPTION_IN_PAGE_ERROR,		  GDB_SIGNAL_SEGV},
+     {EXCEPTION_INVALID_HANDLE, 	  GDB_SIGNAL_SEGV},
+     {EXCEPTION_ILLEGAL_INSTRUCTION, 	  GDB_SIGNAL_ILL},
+     {EXCEPTION_NONCONTINUABLE_EXCEPTION, GDB_SIGNAL_ILL},
+     {EXCEPTION_ARRAY_BOUNDS_EXCEEDED, 	  GDB_SIGNAL_SEGV},
+     {EXCEPTION_FLT_DENORMAL_OPERAND, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_INEXACT_RESULT, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_INVALID_OPERATION, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_OVERFLOW, 		  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_STACK_CHECK, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_FLT_UNDERFLOW, 		  GDB_SIGNAL_FPE},
+     {EXCEPTION_INT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
+     {EXCEPTION_INT_OVERFLOW, 		  GDB_SIGNAL_FPE},
+     {EXCEPTION_PRIV_INSTRUCTION, 	  GDB_SIGNAL_ILL},
+     {EXCEPTION_STACK_OVERFLOW, 	  GDB_SIGNAL_SEGV},
+     {CONTROL_C_EXIT, 			  GDB_SIGNAL_TERM}
+    };
+
+  for (const xlate_status &x : status_xlate_tbl)
+    if (x.status == status)
+      return x.sig;
+
+  return GDB_SIGNAL_UNKNOWN;
+}
+
+#endif	/* __MINGW32__ */

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 028d1e9..41e4ed1 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,12 @@
+2019-12-25  Eli Zaretskii  <eliz@gnu.org>
+
+	* win32-low.c (get_child_debug_event): Extract the fatal exception
+	from the exit status and convert to the equivalent Posix signal
+	number.
+	(win32_wait): Allow TARGET_WAITKIND_SIGNALLED status as well.
+
+	* Makefile.in (OBS, SFILES): Add gdb_wait.[co].
+
 2019-12-19  Christian Biesinger  <cbiesinger@google.com>
 
 	* configure: Regenerate.
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 9e8c213..8022753 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -217,6 +217,7 @@ SFILES = \
 	$(srcdir)/gdbsupport/gdb-dlfcn.c \
 	$(srcdir)/gdbsupport/gdb_tilde_expand.c \
 	$(srcdir)/gdbsupport/gdb_vecs.c \
+	$(srcdir)/gdbsupport/gdb_wait.c \
 	$(srcdir)/gdbsupport/netstuff.c \
 	$(srcdir)/gdbsupport/new-op.c \
 	$(srcdir)/gdbsupport/pathstuff.c \
@@ -264,6 +265,7 @@ OBS = \
 	gdbsupport/gdb-dlfcn.o \
 	gdbsupport/gdb_tilde_expand.o \
 	gdbsupport/gdb_vecs.o \
+	gdbsupport/gdb_wait.o \
 	gdbsupport/netstuff.o \
 	gdbsupport/new-op.o \
 	gdbsupport/pathstuff.o \
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 449ed5f..d6ff794 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -34,6 +34,7 @@
 #include <process.h>
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/common-inferior.h"
+#include "gdbsupport/gdb_wait.h"
 
 #ifndef USE_WIN32API
 #include <sys/cygwin.h>
@@ -1511,8 +1512,24 @@ get_child_debug_event (struct target_waitstatus *ourstatus)
 		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
-      ourstatus->kind = TARGET_WAITKIND_EXITED;
-      ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
+      {
+	DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
+	/* If the exit status looks like a fatal exception, but we
+	   don't recognize the exception's code, make the original
+	   exit status value available, to avoid losing information.  */
+	enum gdb_signal exit_signal = WIFSIGNALED (exit_status)
+	  ? WTERMSIG (exit_status) : GDB_SIGNAL_UNKNOWN;
+	if (exit_signal == GDB_SIGNAL_UNKNOWN)
+	  {
+	    ourstatus->kind = TARGET_WAITKIND_EXITED;
+	    ourstatus->value.integer = exit_status;
+	  }
+	else
+	  {
+	    ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+	    ourstatus->value.sig = exit_signal;
+	  }
+      }
       child_continue (DBG_CONTINUE, -1);
       CloseHandle (current_process_handle);
       current_process_handle = NULL;
@@ -1607,6 +1624,7 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 	  win32_clear_inferiors ();
 	  return ptid_t (current_event.dwProcessId);
 	case TARGET_WAITKIND_STOPPED:
+	case TARGET_WAITKIND_SIGNALLED:
 	case TARGET_WAITKIND_LOADED:
 	  OUTMSG2 (("Child Stopped with signal = %d \n",
 		    ourstatus->value.sig));

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 12/25/19 3:57 PM, Eli Zaretskii wrote:
> No further comments, so I made additional changes to take care of
> Pedro's review comments, and the result is below.

Thanks.  I have some comments, and while thinking through them
I updated the patch accordingly.  See below.

> 
> OK to commit?
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index acf9106..838eafd 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,26 @@
> +2019-12-25  Eli Zaretskii  <eliz@gnu.org>
> +
> +	* windows-tdep.c: New enumeration of WINDOWS_SIG* signals.
> +	(windows_gdb_signal_to_target): New function, uses the above
> +	enumeration to convert GDB internal signal codes to equivalent
> +	Windows codes.
> +	(windows_init_abi): Call set_gdbarch_gdb_signal_to_target.
> +
> +	* windows-nat.c (get_windows_debug_event): Extract the fatal
> +	exception from the exit status and convert to the equivalent Posix
> +	signal number.
> +
> +	* gdbsupport/gdb_wait.c: New file, implements
> +	windows_status_to_termsig.
> +
> +	* gdbsupport/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS)
> +	(WTERMSIG) [__MINGW32__]: Separate definitions for MinGW.
> +
> +	* cli/cli-cmds.c (exit_status_set_internal_vars): Account for the
> +	possibility that WTERMSIG returns GDB_SIGNAL_UNKNOWN.
> +
> +	* Makefile.in (COMMON_SFILES): Add gdbsupport/gdb_wait.c.
> +
>  2019-12-21  George Barrett  <bob@bob131.so>
>  
>  	* solib-svr4.c (svr4_handle_solib_event): Add fallback link
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index fa5c820..34d0acf 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -986,6 +986,7 @@ COMMON_SFILES = \
>  	gdbsupport/gdb-dlfcn.c \
>  	gdbsupport/gdb_tilde_expand.c \
>  	gdbsupport/gdb_vecs.c \
> +	gdbsupport/gdb_wait.c \
>  	gdbsupport/netstuff.c \
>  	gdbsupport/new-op.c \
>  	gdbsupport/pathstuff.c \
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 681d53c..0d1584e 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -798,10 +798,16 @@ exit_status_set_internal_vars (int exit_status)
>  
>    clear_internalvar (var_code);
>    clear_internalvar (var_signal);
> -  if (WIFEXITED (exit_status))
> -    set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
> -  else if (WIFSIGNALED (exit_status))
> +  enum gdb_signal exit_signal
> +    = WIFSIGNALED (exit_status) ? WTERMSIG (exit_status) : GDB_SIGNAL_UNKNOWN;

This is mixing host signals, and enum gdb_signal, which is bad.
It is conceivable that some system uses 143 as a real signal
number, for example.  

Actually, it doesn't even compile on non-MinGW systems.  On GNU/Linux:

src/gdb/cli/cli-cmds.c: In function ‘void exit_status_set_internal_vars(int)’:
src/gdb/cli/cli-cmds.c:802:33: error: invalid conversion from ‘int’ to ‘gdb_signal’ [-fpermissive]
     = WIFSIGNALED (exit_status) ? WTERMSIG (exit_status) : GDB_SIGNAL_UNKNOWN;


This needs to be written in a form that doesn't mix host signals
and enum gdb_signal.  I think the best is to make WTERMSIG emulation
return host signals like Posix, and return -1 in the "unknown" case.

> +  /* The GDB_SIGNAL_UNKNOWN condition can happen on MinGW, if we don't
> +     recognize the fatal exception code encoded in the exit status;
> +     see gdb_wait.c.  We don't want to lose information in the exit
> +     status in that case.  */
> +  if (exit_signal != GDB_SIGNAL_UNKNOWN)
>      set_internalvar_integer (var_signal, WTERMSIG (exit_status));
> +  else if (!WIFSIGNALED (exit_status))
> +    set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
>    else
>      warning (_("unexpected shell command exit status %d"), exit_status);

IIUC, this ends up in "warning" branch in the case where we don't
recognize the exception.  I think it would be better if this were consistent
with how windows-nat.c handles it.  I.e., treat an unrecognized
exception as a normal exit status and record it in $_exitcode.

Like:

 @@ -800,6 +800,18 @@ exit_status_set_internal_vars (int exit_status)
    clear_internalvar (var_signal);
    if (WIFEXITED (exit_status))
      set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
 +#ifdef __MINGW32__
 +  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
 +    {
 +      /* The -1 condition can happen on MinGW, if we don't recognize
 +        the fatal exception code encoded in the exit status; see
 +        gdbsupport/gdb_wait.c.  We don't want to lose information in
 +        the exit status in that case.  Record it as a normal exit
 +        with the full exit status, including the higher 0xC0000000
 +        bits.  */
 +      set_internalvar_integer (var_code, exit_status);
 +    }
 +#endif
    else if (WIFSIGNALED (exit_status))
      set_internalvar_integer (var_signal, WTERMSIG (exit_status));
    else

#ifdef here is OK because this code is only for handling host signals
from programs run on the same machine as GDB.  It is not about handling
signals from the target system.  I.e., cross-debugging isn't a
consideration here.

>  }
> 
> diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
> index 10d5c95..12e50b6 100644
> --- a/gdb/windows-nat.c
> +++ b/gdb/windows-nat.c
> @@ -68,6 +68,7 @@
>  #include "inf-child.h"
>  #include "gdbsupport/gdb_tilde_expand.h"
>  #include "gdbsupport/pathstuff.h"
> +#include "gdbsupport/gdb_wait.h"
>  
>  #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
>  #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
> @@ -1620,8 +1621,22 @@ get_windows_debug_event (struct target_ops *ops,
>  	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
>  					 current_event.dwThreadId),
>  				 0, true /* main_thread_p */);
> -	  ourstatus->kind = TARGET_WAITKIND_EXITED;
> -	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
> +	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
> +	  /* If the exit status looks like a fatal exception, but we
> +	     don't recognize the exception's code, make the original
> +	     exit status value available, to avoid losing information.  */
> +	  enum gdb_signal exit_signal = WIFSIGNALED (exit_status)
> +	    ? WTERMSIG (exit_status) : GDB_SIGNAL_UNKNOWN;

This is native code, so the "some system uses 143 as real signal"
concern doesn't apply, but we should still not use gdb_signal
to hold host signals, it's a design violation.  And it probably doesn't
compile on Cygwin.  Easily adjusted in a similar way to the cli/ code once
WTERMSIG returns a host signal number.  We can then use gdb_signal_from_host
to convert the Windows/host's Posix signal to a gdb_signal, like all
other ports do.

> +	  if (exit_signal == GDB_SIGNAL_UNKNOWN)
> +	    {
> +	      ourstatus->kind = TARGET_WAITKIND_EXITED;
> +	      ourstatus->value.integer = exit_status;
> +	    }
> +	  else
> +	    {
> +	      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
> +	      ourstatus->value.sig = exit_signal;
> +	    }
>  	  thread_id = current_event.dwThreadId;
>  	}
>        break;
> diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> index bb69a79..c2cecce 100644
> --- a/gdb/windows-tdep.c
> +++ b/gdb/windows-tdep.c
> @@ -35,6 +35,55 @@
>  #include "solib-target.h"
>  #include "gdbcore.h"
>  
> +/* Windows signal numbers differ between MinGW flavors and between
> +   those and Cygwin.  The below enumeration was gleaned from the
> +   respective headers; the ones marked with MinGW64/Cygwin are defined
> +   only by MinGW64 and Cygwin, not by mingw.org's MinGW.  */
> +
> +enum
> +  {
> +   WINDOWS_SIGHUP = 1,	/* MinGW64/Cygwin */
> +   WINDOWS_SIGINT = 2,
> +   WINDOWS_SIGQUIT = 3,	/* MinGW64/Cygwin */
> +   WINDOWS_SIGILL = 4,
> +   WINDOWS_SIGTRAP = 5,	/* MinGW64/Cygwin */
> +#ifdef __CYGWIN__
> +   WINDOWS_SGABRT = 6,
> +#else
> +   WINDOWS_SIGIOT = 6,	/* MinGW64 */
> +#endif
> +   WINDOWS_SIGEMT = 7,	/* MinGW64/Cygwin */
> +   WINDOWS_SIGFPE = 8,
> +   WINDOWS_SIGKILL = 9,	/* MinGW64/Cygwin */
> +   WINDOWS_SIGBUS = 10,	/* MinGW64/Cygwin */
> +   WINDOWS_SIGSEGV = 11,
> +   WINDOWS_SIGSYS = 12,	/* MinGW64/Cygwin */
> +   WINDOWS_SIGPIPE = 13,/* MinGW64/Cygwin */
> +   WINDOWS_SIGALRM = 14,/* MinGW64/Cygwin */
> +   WINDOWS_SIGTERM = 15,
> +#ifdef __CYGWIN__
> +   WINDOWS_SIGURG = 16,
> +   WINDOWS_SIGSTOP = 17,
> +   WINDOWS_SIGTSTP = 18,
> +   WINDOWS_SIGCONT = 19,
> +   WINDOWS_SIGCHLD = 20,
> +   WINDOWS_SIGTTIN = 21,
> +   WINDOWS_SIGTTOU = 22,
> +   WINDOWS_SIGIO = 23,
> +   WINDOWS_SIGXCPU = 24,
> +   WINDOWS_SIGXFSZ = 25,
> +   WINDOWS_SIGVTALRM = 26,
> +   WINDOWS_SIGPROF = 27,
> +   WINDOWS_SIGWINCH = 28,
> +   WINDOWS_SIGLOST = 29,
> +   WINDOWS_SIGUSR1 = 30,
> +   WINDOWS_SIGUSR2 = 31
> +#else
> +   WINDOWS_SIGBREAK = 21,
> +   WINDOWS_SIGABRT = 22
> +#endif

Bummer on the #ifdefs.  This is still incorrect design-wise, because the
values depend on how GDB is built.  We must not do that in tdep files,
so that cross debugging works correctly.  I.e., a Cygwin gdb debugging
against a MinGW gdbserver should understand MinGW signals, and vice versa.
Or a Cygwin GDB debugging a MinGW program natively too.  

From <https://sourceware.org/gdb/wiki/Internals/Source%20Tree%20Structure>:

 "Since these files are used when you configure GDB as a cross debugger (e.g.,
 a GDB that runs on x86 but is configured to debug ARM code), all code in
 these files is host-independent. E.g., including host-specific headers,
 assuming host endianness, etc. in these files would be incorrect."

It is a known limitation that GDB currently doesn't have separate "set osabi"
awareness for Cygwin vs MinGW, so I'll let this pass, with a FIXME note.
See <https://sourceware.org/bugzilla/show_bug.cgi?id=21500#c1>.

> +  };
> +
>  struct cmd_list_element *info_w32_cmdlist;
>  
>  typedef struct thread_information_block_32
> @@ -461,6 +510,81 @@ init_w32_command_list (void)
>      }
>  }
>  
> +static int
> +windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)

I've added the conventional intro comment.

> +{
> +  switch (signal)
> +    {
> +    case GDB_SIGNAL_0:
> +      return 0;
> +    case GDB_SIGNAL_HUP:
> +      return WINDOWS_SIGHUP;
> +    case GDB_SIGNAL_INT:
> +      return WINDOWS_SIGINT;
> +    case GDB_SIGNAL_QUIT:
> +      return WINDOWS_SIGQUIT;
> +    case GDB_SIGNAL_ILL:
> +      return WINDOWS_SIGILL;
> +    case GDB_SIGNAL_TRAP:
> +      return WINDOWS_SIGTRAP;
> +    case GDB_SIGNAL_ABRT:
> +      return WINDOWS_SIGABRT;
> +    case GDB_SIGNAL_EMT:
> +      return WINDOWS_SIGEMT;
> +    case GDB_SIGNAL_FPE:
> +      return WINDOWS_SIGFPE;
> +    case GDB_SIGNAL_KILL:
> +      return WINDOWS_SIGKILL;
> +    case GDB_SIGNAL_BUS:
> +      return WINDOWS_SIGBUS;
> +    case GDB_SIGNAL_SEGV:
> +      return WINDOWS_SIGSEGV;
> +    case GDB_SIGNAL_SYS:
> +      return WINDOWS_SIGSYS;
> +    case GDB_SIGNAL_PIPE:
> +      return WINDOWS_SIGPIPE;
> +    case GDB_SIGNAL_ALRM:
> +      return WINDOWS_SIGALRM;
> +    case GDB_SIGNAL_TERM:
> +      return WINDOWS_SIGTERM;
> +#ifdef __CYGWIN__
> +    case GDB_SIGNAL_URG:
> +      return WINDOWS_SIGURG;
> +    case GDB_SIGNAL_STOP:
> +      return WINDOWS_SIGSTOP;
> +    case GDB_SIGNAL_TSTP:
> +      return WINDOWS_SIGTSTP;
> +    case GDB_SIGNAL_CONT:
> +      return WINDOWS_SIGCONT;
> +    case GDB_SIGNAL_CHLD:
> +      return WINDOWS_SIGCHLD;
> +    case GDB_SIGNAL_TTIN:
> +      return WINDOWS_SIGTTIN;
> +    case GDB_SIGNAL_TTOU:
> +      return WINDOWS_SIGTTOU;
> +    case GDB_SIGNAL_IO:
> +      return WINDOWS_SIGIO;
> +    case GDB_SIGNAL_XCPU:
> +      return WINDOWS_SIGXCPU;
> +    case GDB_SIGNAL_XFSZ:
> +      return WINDOWS_SIGXFSZ;
> +    case GDB_SIGNAL_VTALRM:
> +      return WINDOWS_SIGVTALRM;
> +    case GDB_SIGNAL_PROF:
> +      return WINDOWS_SIGPROF;
> +    case GDB_SIGNAL_WINCH:
> +      return WINDOWS_SIGWINCH;
> +    case GDB_SIGNAL_PWR:
> +      return WINDOWS_SIGLOST;
> +    case GDB_SIGNAL_USR1:
> +      return WINDOWS_SIGUSR1;
> +    case GDB_SIGNAL_USR2:
> +      return WINDOWS_SIGUSR2;
> +#endif	/* __CYGWIN__ */
> +    }
> +  return -1;
> +}
> +
>  /* To be called from the various GDB_OSABI_CYGWIN handlers for the
>     various Windows architectures and machine types.  */
>  
> @@ -477,6 +601,8 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    set_gdbarch_iterate_over_objfiles_in_search_order
>      (gdbarch, windows_iterate_over_objfiles_in_search_order);
>  
> +  set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target);
> +
>    set_solib_ops (gdbarch, &solib_target_so_ops);
>  }
>  
> 
> diff --git a/gdb/gdbsupport/gdb_wait.h b/gdb/gdbsupport/gdb_wait.h
> index b3b752c..e8597f9 100644
> --- a/gdb/gdbsupport/gdb_wait.h
> +++ b/gdb/gdbsupport/gdb_wait.h
> @@ -38,20 +38,33 @@
>     in POSIX.1.  We fail to define WNOHANG and WUNTRACED, which POSIX.1
>     <sys/wait.h> defines, since our code does not use waitpid() (but
>     NOTE exception for GNU/Linux below).  We also fail to declare
> -   wait() and waitpid().  */
> +   wait() and waitpid().
> +
> +   For MinGW, we use the fact that when a Windows program is
> +   terminated by a fatal exception, its exit code is the value of that
> +   exception, as defined by the various EXCEPTION_* symbols in the
> +   Windows API headers.  See also gdb_wait.c.  */
>  
>  #ifndef	WIFEXITED
> -#define WIFEXITED(w)	(((w)&0377) == 0)
> +# ifdef __MINGW32__
> +#  define WIFEXITED(w)	(((w) & 0xC0000000) == 0)
> +# else
> +#  define WIFEXITED(w)	(((w)&0377) == 0)
> +# endif
>  #endif
>  
>  #ifndef	WIFSIGNALED
> -#define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
> +# ifdef __MINGW32__
> +#  define WIFSIGNALED(w)	(((w) & 0xC0000000) == 0xC0000000)
> +# else
> +#  define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
> +# endif
>  #endif
>  
>  #ifndef	WIFSTOPPED
>  #ifdef IBM6000
>  
> -/* Unfortunately, the above comment (about being compatible in all Unix 
> +/* Unfortunately, the above comment (about being compatible in all Unix
>     systems) is not quite correct for AIX, sigh.  And AIX 3.2 can generate
>     status words like 0x57c (sigtrap received after load), and gdb would
>     choke on it.  */
> @@ -64,11 +77,20 @@
>  #endif
>  
>  #ifndef	WEXITSTATUS
> -#define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
> +# ifdef __MINGW32__
> +#  define WEXITSTATUS(w)	((w) & ~0xC0000000)

I've noticed that this clearing of the 0xC0 bits isn't really necessary,
since you're only supposed to call this if WIFEXITED returns true.
I've left it alone but I wonder whether we shouldn't remove that, since
I can't figure out when we'd really want to strip those bits.

> +# else
> +#  define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
> +# endif
>  #endif
>  
>  #ifndef	WTERMSIG
> -#define WTERMSIG(w)	((w) & 0177)
> +# ifdef __MINGW32__
> +extern enum gdb_signal windows_status_to_termsig (unsigned long);

This here should return int.

> +#  define WTERMSIG(w)	windows_status_to_termsig (w)
> +# else
> +#  define WTERMSIG(w)	((w) & 0177)
> +# endif
>  #endif
>  
>  #ifndef	WSTOPSIG
> 
>  
> diff --git a/gdb/gdbsupport/gdb_wait.c b/gdb/gdbsupport/gdb_wait.c
> new file mode 100644
> index 0000000..7096152
> --- /dev/null
> +++ b/gdb/gdbsupport/gdb_wait.c
> @@ -0,0 +1,80 @@
> +/* Support code for standard wait macros in gdb_wait.h.
> +
> +   Copyright (C) 2019 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 "common-defs.h"
> +
> +#ifdef __MINGW32__
> +
> +/* The underlying idea is that when a Windows program is terminated by
> +   a fatal exception, its exit code is the value of that exception, as
> +   defined by the various EXCEPTION_* symbols in the Windows API
> +   headers.  We thus emulate WTERMSIG etc. by translating the fatal
> +   exception codes to more-or-less equivalent Posix signals.
> +
> +   The translation below is not perfect, because a program could
> +   legitimately exit normally with a status whose value happens to
> +   have the high bits set, but that's extremely rare, to say the
> +   least, and it is deemed such a negligibly small probability of
> +   false positives is justified by the utility of reporting the
> +   terminating signal in the "normal" cases.  */
> +
> +# include "gdb/signals.h"	/* for enum gdb_signal */
> +
> +# define WIN32_LEAN_AND_MEAN
> +# include <windows.h>		/* for EXCEPTION_* constants */
> +
> +struct xlate_status
> +{
> +  DWORD status;		/* exit status (actually, fatal exception code) */
> +  enum gdb_signal sig;	/* corresponding GDB signal value */
> +};
> +
> +enum gdb_signal
> +windows_status_to_termsig (unsigned long status)

See the new version in the new patch.  It's the same, except it
returns host signals SIGSEGV, SIGILL, etc., and -1 on "unknown".

> +{
> +  static const xlate_status status_xlate_tbl[] =
> +    {
> +     {EXCEPTION_ACCESS_VIOLATION, 	  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_IN_PAGE_ERROR,		  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_INVALID_HANDLE, 	  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_ILLEGAL_INSTRUCTION, 	  GDB_SIGNAL_ILL},
> +     {EXCEPTION_NONCONTINUABLE_EXCEPTION, GDB_SIGNAL_ILL},
> +     {EXCEPTION_ARRAY_BOUNDS_EXCEEDED, 	  GDB_SIGNAL_SEGV},
> +     {EXCEPTION_FLT_DENORMAL_OPERAND, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_INEXACT_RESULT, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_INVALID_OPERATION, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_OVERFLOW, 		  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_STACK_CHECK, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_FLT_UNDERFLOW, 		  GDB_SIGNAL_FPE},
> +     {EXCEPTION_INT_DIVIDE_BY_ZERO, 	  GDB_SIGNAL_FPE},
> +     {EXCEPTION_INT_OVERFLOW, 		  GDB_SIGNAL_FPE},
> +     {EXCEPTION_PRIV_INSTRUCTION, 	  GDB_SIGNAL_ILL},
> +     {EXCEPTION_STACK_OVERFLOW, 	  GDB_SIGNAL_SEGV},
> +     {CONTROL_C_EXIT, 			  GDB_SIGNAL_TERM}
> +    };
> +
> +  for (const xlate_status &x : status_xlate_tbl)
> +    if (x.status == status)
> +      return x.sig;
> +
> +  return GDB_SIGNAL_UNKNOWN;
> +}
> +
Here's the updated patch.  I confirmed that it builds on MinGW-W64
using my Fedora's cross cross compiler, but I didn't try to run
the resulting GDB (since I'm not on Windows).

WDYT?  Does it work for you?  I think this should have a NEWS
entry, BTW.

From dce7b34af6c252556a83dce310eb546cc6588285 Mon Sep 17 00:00:00 2001
From: Eli Zaretskii <eliz@gnu.org>
Date: Fri, 3 Jan 2020 19:30:15 +0000
Subject: [PATCH] Improve process exit status macros on MinGW

FIXME: Missing git commit log & gdb/NEWS entry.

gdb/ChangeLog:
yyyy-mm-dd  Eli Zaretskii  <eliz@gnu.org>
	    Pedro Alves  <palves@redhat.com>

	* Makefile.in (COMMON_SFILES): Add gdbsupport/gdb_wait.c.
	* windows-tdep.c: New enumeration of WINDOWS_SIG* signals.
	(windows_gdb_signal_to_target): New function, uses the above
	enumeration to convert GDB internal signal codes to equivalent
	Windows codes.
	(windows_init_abi): Call set_gdbarch_gdb_signal_to_target.
	* windows-nat.c: Include "gdb_wait.h".
	(get_windows_debug_event): Extract the fatal exception from the
	exit status and convert to the equivalent Posix signal number.
	* cli/cli-cmds.c (exit_status_set_internal_vars): Account for the
	possibility that WTERMSIG returns GDB_SIGNAL_UNKNOWN.
	* gdbsupport/gdb_wait.c: New file, implements
	windows_status_to_termsig.
	* gdbsupport/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS)
	(WTERMSIG) [__MINGW32__]: Separate definitions for MinGW.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Eli Zaretskii  <eliz@gnu.org>
	    Pedro Alves  <palves@redhat.com>

	* win32-low.c (get_child_debug_event): Extract the fatal exception
	from the exit status and convert to the equivalent Posix signal
	number.
	(win32_wait): Allow TARGET_WAITKIND_SIGNALLED status as well.
	* Makefile.in (OBS, SFILES): Add gdb_wait.[co].
---
 gdb/Makefile.in           |   1 +
 gdb/cli/cli-cmds.c        |  12 +++++
 gdb/gdbserver/Makefile.in |   2 +
 gdb/gdbserver/win32-low.c |  22 +++++++-
 gdb/gdbsupport/gdb_wait.c |  83 +++++++++++++++++++++++++++++
 gdb/gdbsupport/gdb_wait.h |  34 +++++++++---
 gdb/windows-nat.c         |  20 ++++++-
 gdb/windows-tdep.c        | 130 ++++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 294 insertions(+), 10 deletions(-)
 create mode 100644 gdb/gdbsupport/gdb_wait.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 448a495bb3b..0bc8142d2a0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -986,6 +986,7 @@ COMMON_SFILES = \
 	gdbsupport/gdb-dlfcn.c \
 	gdbsupport/gdb_tilde_expand.c \
 	gdbsupport/gdb_vecs.c \
+	gdbsupport/gdb_wait.c \
 	gdbsupport/netstuff.c \
 	gdbsupport/new-op.c \
 	gdbsupport/pathstuff.c \
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 0b55a1244aa..c2ba3a4238a 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -800,6 +800,18 @@ exit_status_set_internal_vars (int exit_status)
   clear_internalvar (var_signal);
   if (WIFEXITED (exit_status))
     set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
+#ifdef __MINGW32__
+  else if (WIFSIGNALED (exit_status) && WTERMSIG (exit_status) == -1)
+    {
+      /* The -1 condition can happen on MinGW, if we don't recognize
+	 the fatal exception code encoded in the exit status; see
+	 gdbsupport/gdb_wait.c.  We don't want to lose information in
+	 the exit status in that case.  Record it as a normal exit
+	 with the full exit status, including the higher 0xC0000000
+	 bits.  */
+      set_internalvar_integer (var_code, exit_status);
+    }
+#endif
   else if (WIFSIGNALED (exit_status))
     set_internalvar_integer (var_signal, WTERMSIG (exit_status));
   else
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index d39c065f6d0..1125426778b 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -217,6 +217,7 @@ SFILES = \
 	$(srcdir)/gdbsupport/gdb-dlfcn.c \
 	$(srcdir)/gdbsupport/gdb_tilde_expand.c \
 	$(srcdir)/gdbsupport/gdb_vecs.c \
+	$(srcdir)/gdbsupport/gdb_wait.c \
 	$(srcdir)/gdbsupport/netstuff.c \
 	$(srcdir)/gdbsupport/new-op.c \
 	$(srcdir)/gdbsupport/pathstuff.c \
@@ -264,6 +265,7 @@ OBS = \
 	gdbsupport/gdb-dlfcn.o \
 	gdbsupport/gdb_tilde_expand.o \
 	gdbsupport/gdb_vecs.o \
+	gdbsupport/gdb_wait.o \
 	gdbsupport/netstuff.o \
 	gdbsupport/new-op.o \
 	gdbsupport/pathstuff.o \
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index b4ee0f45c4f..340f65bbf95 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -34,6 +34,7 @@
 #include <process.h>
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/common-inferior.h"
+#include "gdbsupport/gdb_wait.h"
 
 #ifndef USE_WIN32API
 #include <sys/cygwin.h>
@@ -1511,8 +1512,24 @@ get_child_debug_event (struct target_waitstatus *ourstatus)
 		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
-      ourstatus->kind = TARGET_WAITKIND_EXITED;
-      ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
+      {
+	DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
+	/* If the exit status looks like a fatal exception, but we
+	   don't recognize the exception's code, make the original
+	   exit status value available, to avoid losing information.  */
+	int exit_signal
+	  = WIFSIGNALED (exit_status) ? WTERMSIG (exit_status) : -1;
+	if (exit_signal == -1)
+	  {
+	    ourstatus->kind = TARGET_WAITKIND_EXITED;
+	    ourstatus->value.integer = exit_status;
+	  }
+	else
+	  {
+	    ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+	    ourstatus->value.sig = gdb_signal_from_host (exit_signal);
+	  }
+      }
       child_continue (DBG_CONTINUE, -1);
       CloseHandle (current_process_handle);
       current_process_handle = NULL;
@@ -1607,6 +1624,7 @@ win32_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
 	  win32_clear_inferiors ();
 	  return ptid_t (current_event.dwProcessId);
 	case TARGET_WAITKIND_STOPPED:
+	case TARGET_WAITKIND_SIGNALLED:
 	case TARGET_WAITKIND_LOADED:
 	  OUTMSG2 (("Child Stopped with signal = %d \n",
 		    ourstatus->value.sig));
diff --git a/gdb/gdbsupport/gdb_wait.c b/gdb/gdbsupport/gdb_wait.c
new file mode 100644
index 00000000000..037ba643db4
--- /dev/null
+++ b/gdb/gdbsupport/gdb_wait.c
@@ -0,0 +1,83 @@
+/* Support code for standard wait macros in gdb_wait.h.
+
+   Copyright (C) 2019 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 "common-defs.h"
+
+#ifdef __MINGW32__
+
+/* The underlying idea is that when a Windows program is terminated by
+   a fatal exception, its exit code is the value of that exception, as
+   defined by the various EXCEPTION_* symbols in the Windows API
+   headers.  We thus emulate WTERMSIG etc. by translating the fatal
+   exception codes to more-or-less equivalent Posix signals.
+
+   The translation below is not perfect, because a program could
+   legitimately exit normally with a status whose value happens to
+   have the high bits set, but that's extremely rare, to say the
+   least, and it is deemed such a negligibly small probability of
+   false positives is justified by the utility of reporting the
+   terminating signal in the "normal" cases.  */
+
+# include "gdb/signals.h"	/* for enum gdb_signal */
+
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>		/* for EXCEPTION_* constants */
+
+struct xlate_status
+{
+  /* The exit status (actually, fatal exception code).  */
+  DWORD status;
+
+  /* The corresponding signal value.  */
+  int sig;
+};
+
+int
+windows_status_to_termsig (unsigned long status)
+{
+  static const xlate_status status_xlate_tbl[] =
+    {
+     {EXCEPTION_ACCESS_VIOLATION,	  SIGSEGV},
+     {EXCEPTION_IN_PAGE_ERROR,		  SIGSEGV},
+     {EXCEPTION_INVALID_HANDLE,		  SIGSEGV},
+     {EXCEPTION_ILLEGAL_INSTRUCTION,	  SIGILL},
+     {EXCEPTION_NONCONTINUABLE_EXCEPTION, SIGILL},
+     {EXCEPTION_ARRAY_BOUNDS_EXCEEDED,	  SIGSEGV},
+     {EXCEPTION_FLT_DENORMAL_OPERAND,	  SIGFPE},
+     {EXCEPTION_FLT_DIVIDE_BY_ZERO,	  SIGFPE},
+     {EXCEPTION_FLT_INEXACT_RESULT,	  SIGFPE},
+     {EXCEPTION_FLT_INVALID_OPERATION,	  SIGFPE},
+     {EXCEPTION_FLT_OVERFLOW,		  SIGFPE},
+     {EXCEPTION_FLT_STACK_CHECK,	  SIGFPE},
+     {EXCEPTION_FLT_UNDERFLOW,		  SIGFPE},
+     {EXCEPTION_INT_DIVIDE_BY_ZERO,	  SIGFPE},
+     {EXCEPTION_INT_OVERFLOW,		  SIGFPE},
+     {EXCEPTION_PRIV_INSTRUCTION,	  SIGILL},
+     {EXCEPTION_STACK_OVERFLOW,		  SIGSEGV},
+     {CONTROL_C_EXIT,			  SIGTERM}
+    };
+
+  for (const xlate_status &x : status_xlate_tbl)
+    if (x.status == status)
+      return x.sig;
+
+  return -1;
+}
+
+#endif	/* __MINGW32__ */
diff --git a/gdb/gdbsupport/gdb_wait.h b/gdb/gdbsupport/gdb_wait.h
index d00b4592051..3563b9cb954 100644
--- a/gdb/gdbsupport/gdb_wait.h
+++ b/gdb/gdbsupport/gdb_wait.h
@@ -38,20 +38,33 @@
    in POSIX.1.  We fail to define WNOHANG and WUNTRACED, which POSIX.1
    <sys/wait.h> defines, since our code does not use waitpid() (but
    NOTE exception for GNU/Linux below).  We also fail to declare
-   wait() and waitpid().  */
+   wait() and waitpid().
+
+   For MinGW, we use the fact that when a Windows program is
+   terminated by a fatal exception, its exit code is the value of that
+   exception, as defined by the various EXCEPTION_* symbols in the
+   Windows API headers.  See also gdb_wait.c.  */
 
 #ifndef	WIFEXITED
-#define WIFEXITED(w)	(((w)&0377) == 0)
+# ifdef __MINGW32__
+#  define WIFEXITED(w)	(((w) & 0xC0000000) == 0)
+# else
+#  define WIFEXITED(w)	(((w)&0377) == 0)
+# endif
 #endif
 
 #ifndef	WIFSIGNALED
-#define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
+# ifdef __MINGW32__
+#  define WIFSIGNALED(w)	(((w) & 0xC0000000) == 0xC0000000)
+# else
+#  define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
+# endif
 #endif
 
 #ifndef	WIFSTOPPED
 #ifdef IBM6000
 
-/* Unfortunately, the above comment (about being compatible in all Unix 
+/* Unfortunately, the above comment (about being compatible in all Unix
    systems) is not quite correct for AIX, sigh.  And AIX 3.2 can generate
    status words like 0x57c (sigtrap received after load), and gdb would
    choke on it.  */
@@ -64,11 +77,20 @@
 #endif
 
 #ifndef	WEXITSTATUS
-#define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
+# ifdef __MINGW32__
+#  define WEXITSTATUS(w)	((w) & ~0xC0000000)
+# else
+#  define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
+# endif
 #endif
 
 #ifndef	WTERMSIG
-#define WTERMSIG(w)	((w) & 0177)
+# ifdef __MINGW32__
+extern int windows_status_to_termsig (unsigned long);
+#  define WTERMSIG(w)	windows_status_to_termsig (w)
+# else
+#  define WTERMSIG(w)	((w) & 0177)
+# endif
 #endif
 
 #ifndef	WSTOPSIG
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 2214caacb81..b4cb5eec6cc 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -68,6 +68,7 @@
 #include "inf-child.h"
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/pathstuff.h"
+#include "gdbsupport/gdb_wait.h"
 
 #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
 #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
@@ -1627,8 +1628,23 @@ get_windows_debug_event (struct target_ops *ops,
 	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
 					 current_event.dwThreadId),
 				 0, true /* main_thread_p */);
-	  ourstatus->kind = TARGET_WAITKIND_EXITED;
-	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
+	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
+	  /* If the exit status looks like a fatal exception, but we
+	     don't recognize the exception's code, make the original
+	     exit status value available, to avoid losing
+	     information.  */
+	  int exit_signal
+	    = WIFSIGNALED (exit_status) ? WTERMSIG (exit_status) : -1;
+	  if (exit_signal == -1)
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_EXITED;
+	      ourstatus->value.integer = exit_status;
+	    }
+	  else
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+	      ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (w));
+	    }
 	  thread_id = current_event.dwThreadId;
 	}
       break;
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index ca9b81df298..58f8838b885 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -35,6 +35,57 @@
 #include "solib-target.h"
 #include "gdbcore.h"
 
+/* Windows signal numbers differ between MinGW flavors and between
+   those and Cygwin.  The below enumeration was gleaned from the
+   respective headers; the ones marked with MinGW64/Cygwin are defined
+   only by MinGW64 and Cygwin, not by mingw.org's MinGW.  FIXME: We
+   should really have distinct MinGW vs Cygwin OSABIs, and two
+   separate enums, selected at runtime.  */
+
+enum
+  {
+   WINDOWS_SIGHUP = 1,	/* MinGW64/Cygwin */
+   WINDOWS_SIGINT = 2,
+   WINDOWS_SIGQUIT = 3,	/* MinGW64/Cygwin */
+   WINDOWS_SIGILL = 4,
+   WINDOWS_SIGTRAP = 5,	/* MinGW64/Cygwin */
+#ifdef __CYGWIN__
+   WINDOWS_SGABRT = 6,
+#else
+   WINDOWS_SIGIOT = 6,	/* MinGW64 */
+#endif
+   WINDOWS_SIGEMT = 7,	/* MinGW64/Cygwin */
+   WINDOWS_SIGFPE = 8,
+   WINDOWS_SIGKILL = 9,	/* MinGW64/Cygwin */
+   WINDOWS_SIGBUS = 10,	/* MinGW64/Cygwin */
+   WINDOWS_SIGSEGV = 11,
+   WINDOWS_SIGSYS = 12,	/* MinGW64/Cygwin */
+   WINDOWS_SIGPIPE = 13,/* MinGW64/Cygwin */
+   WINDOWS_SIGALRM = 14,/* MinGW64/Cygwin */
+   WINDOWS_SIGTERM = 15,
+#ifdef __CYGWIN__
+   WINDOWS_SIGURG = 16,
+   WINDOWS_SIGSTOP = 17,
+   WINDOWS_SIGTSTP = 18,
+   WINDOWS_SIGCONT = 19,
+   WINDOWS_SIGCHLD = 20,
+   WINDOWS_SIGTTIN = 21,
+   WINDOWS_SIGTTOU = 22,
+   WINDOWS_SIGIO = 23,
+   WINDOWS_SIGXCPU = 24,
+   WINDOWS_SIGXFSZ = 25,
+   WINDOWS_SIGVTALRM = 26,
+   WINDOWS_SIGPROF = 27,
+   WINDOWS_SIGWINCH = 28,
+   WINDOWS_SIGLOST = 29,
+   WINDOWS_SIGUSR1 = 30,
+   WINDOWS_SIGUSR2 = 31
+#else
+   WINDOWS_SIGBREAK = 21,
+   WINDOWS_SIGABRT = 22
+#endif
+  };
+
 struct cmd_list_element *info_w32_cmdlist;
 
 typedef struct thread_information_block_32
@@ -461,6 +512,83 @@ init_w32_command_list (void)
     }
 }
 
+/* Implementation of `gdbarch_gdb_signal_to_target'.  */
+
+static int
+windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
+{
+  switch (signal)
+    {
+    case GDB_SIGNAL_0:
+      return 0;
+    case GDB_SIGNAL_HUP:
+      return WINDOWS_SIGHUP;
+    case GDB_SIGNAL_INT:
+      return WINDOWS_SIGINT;
+    case GDB_SIGNAL_QUIT:
+      return WINDOWS_SIGQUIT;
+    case GDB_SIGNAL_ILL:
+      return WINDOWS_SIGILL;
+    case GDB_SIGNAL_TRAP:
+      return WINDOWS_SIGTRAP;
+    case GDB_SIGNAL_ABRT:
+      return WINDOWS_SIGABRT;
+    case GDB_SIGNAL_EMT:
+      return WINDOWS_SIGEMT;
+    case GDB_SIGNAL_FPE:
+      return WINDOWS_SIGFPE;
+    case GDB_SIGNAL_KILL:
+      return WINDOWS_SIGKILL;
+    case GDB_SIGNAL_BUS:
+      return WINDOWS_SIGBUS;
+    case GDB_SIGNAL_SEGV:
+      return WINDOWS_SIGSEGV;
+    case GDB_SIGNAL_SYS:
+      return WINDOWS_SIGSYS;
+    case GDB_SIGNAL_PIPE:
+      return WINDOWS_SIGPIPE;
+    case GDB_SIGNAL_ALRM:
+      return WINDOWS_SIGALRM;
+    case GDB_SIGNAL_TERM:
+      return WINDOWS_SIGTERM;
+#ifdef __CYGWIN__
+    case GDB_SIGNAL_URG:
+      return WINDOWS_SIGURG;
+    case GDB_SIGNAL_STOP:
+      return WINDOWS_SIGSTOP;
+    case GDB_SIGNAL_TSTP:
+      return WINDOWS_SIGTSTP;
+    case GDB_SIGNAL_CONT:
+      return WINDOWS_SIGCONT;
+    case GDB_SIGNAL_CHLD:
+      return WINDOWS_SIGCHLD;
+    case GDB_SIGNAL_TTIN:
+      return WINDOWS_SIGTTIN;
+    case GDB_SIGNAL_TTOU:
+      return WINDOWS_SIGTTOU;
+    case GDB_SIGNAL_IO:
+      return WINDOWS_SIGIO;
+    case GDB_SIGNAL_XCPU:
+      return WINDOWS_SIGXCPU;
+    case GDB_SIGNAL_XFSZ:
+      return WINDOWS_SIGXFSZ;
+    case GDB_SIGNAL_VTALRM:
+      return WINDOWS_SIGVTALRM;
+    case GDB_SIGNAL_PROF:
+      return WINDOWS_SIGPROF;
+    case GDB_SIGNAL_WINCH:
+      return WINDOWS_SIGWINCH;
+    case GDB_SIGNAL_PWR:
+      return WINDOWS_SIGLOST;
+    case GDB_SIGNAL_USR1:
+      return WINDOWS_SIGUSR1;
+    case GDB_SIGNAL_USR2:
+      return WINDOWS_SIGUSR2;
+#endif	/* __CYGWIN__ */
+    }
+  return -1;
+}
+
 /* To be called from the various GDB_OSABI_CYGWIN handlers for the
    various Windows architectures and machine types.  */
 
@@ -477,6 +605,8 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_iterate_over_objfiles_in_search_order
     (gdbarch, windows_iterate_over_objfiles_in_search_order);
 
+  set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target);
+
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
 

base-commit: 44f81a76542dbeada2541a05de191ae0ac0fbc2c
-- 
2.14.5

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 1/3/20 7:59 PM, Pedro Alves wrote:

>  #define AdjustTokenPrivileges		dyn_AdjustTokenPrivileges
>  #define DebugActiveProcessStop		dyn_DebugActiveProcessStop
> @@ -1627,8 +1628,23 @@ get_windows_debug_event (struct target_ops *ops,
>  	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
>  					 current_event.dwThreadId),
>  				 0, true /* main_thread_p */);
> -	  ourstatus->kind = TARGET_WAITKIND_EXITED;
> -	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
> +	  DWORD exit_status = current_event.u.ExitProcess.dwExitCode;
> +	  /* If the exit status looks like a fatal exception, but we
> +	     don't recognize the exception's code, make the original
> +	     exit status value available, to avoid losing
> +	     information.  */
> +	  int exit_signal
> +	    = WIFSIGNALED (exit_status) ? WTERMSIG (exit_status) : -1;
> +	  if (exit_signal == -1)
> +	    {
> +	      ourstatus->kind = TARGET_WAITKIND_EXITED;
> +	      ourstatus->value.integer = exit_status;
> +	    }
> +	  else
> +	    {
> +	      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
> +	      ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (w));

I notice now that this should be:

    ourstatus->value.sig = gdb_signal_from_host (exit_signal);

I had fixed that on gdbserver, but not on windows-nat.c.  And the reason
I didn't notice is that the build tree I was using is only building
gdbserver, not the whole of gdb + gdbserver, and I failed to notice it.
Hopefully the rest of the code will build cleanly without requiring
much more fixing.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 3 Jan 2020 19:59:22 +0000
> 
> Here's the updated patch.  I confirmed that it builds on MinGW-W64
> using my Fedora's cross cross compiler, but I didn't try to run
> the resulting GDB (since I'm not on Windows).

LGTM, thanks.  Feel free to push.

> I think this should have a NEWS entry, BTW.

It's a kind of a bugfix, so I didn't think it was NEWS-worthy.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 1/3/20 8:34 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 3 Jan 2020 19:59:22 +0000
>>
>> Here's the updated patch.  I confirmed that it builds on MinGW-W64
>> using my Fedora's cross cross compiler, but I didn't try to run
>> the resulting GDB (since I'm not on Windows).
> 
> LGTM, thanks.  Feel free to push.

Pushed.

> 
>> I think this should have a NEWS entry, BTW.
> 
> It's a kind of a bugfix, so I didn't think it was NEWS-worthy.

My thinking is that users that might have been scripting
around $_exitcode may need to adjust to look at $_exitsignal.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 6 Jan 2020 11:57:42 +0000
> 
> >> I think this should have a NEWS entry, BTW.
> > 
> > It's a kind of a bugfix, so I didn't think it was NEWS-worthy.
> 
> My thinking is that users that might have been scripting
> around $_exitcode may need to adjust to look at $_exitsignal.

Fair enough.  How about the below?

diff --git a/gdb/NEWS b/gdb/NEWS
index f51a989..11e4221 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 9
 
+* Debugging MS-Windows processes now sets $_exitsignal when the
+  inferior is terminated by a signal, instead of setting $_exitcode.
+
 * Multithreaded symbol loading has now been enabled by default on systems
   that support it (see entry for GDB 9, below), providing faster
   performance for programs with many symbols.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 1/6/20 4:17 PM, Eli Zaretskii wrote:
>> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Mon, 6 Jan 2020 11:57:42 +0000
>>
>>>> I think this should have a NEWS entry, BTW.
>>>
>>> It's a kind of a bugfix, so I didn't think it was NEWS-worthy.
>>
>> My thinking is that users that might have been scripting
>> around $_exitcode may need to adjust to look at $_exitsignal.
> 
> Fair enough.  How about the below?

LGTM.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Mon, 6 Jan 2020 18:51:20 +0000
> 
> > Fair enough.  How about the below?
> 
> LGTM.

Thanks, pushed.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Hannes Domani via gdb-patches]

 Am Freitag, 3. Januar 2020, 20:59:45 MEZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben:

> See the new version in the new patch.  It's the same, except it
> returns host signals SIGSEGV, SIGILL, etc., and -1 on "unknown".

> Here's the updated patch.  I confirmed that it builds on MinGW-W64
> using my Fedora's cross cross compiler, but I didn't try to run
> the resulting GDB (since I'm not on Windows).
>
> WDYT?  Does it work for you?  I think this should have a NEWS
> entry, BTW.

I just tried to compile current master (with a native mingw-w64 compiler), and have this failure:

C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c: In function 'int windows_status_to_termsig(long unsigned int)':
C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c:56:37: error: 'SIGSEGV' was not declared in this scope
   56 |      {EXCEPTION_ACCESS_VIOLATION,   SIGSEGV},
      |                                     ^~~~~~~
C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c:59:40: error: 'SIGILL' was not declared in this scope
   59 |      {EXCEPTION_ILLEGAL_INSTRUCTION,   SIGILL},
      |                                        ^~~~~~
C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c:62:41: error: 'SIGFPE' was not declared in this scope
   62 |      {EXCEPTION_FLT_DENORMAL_OPERAND,   SIGFPE},
      |                                         ^~~~~~
C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c:73:27: error: 'SIGTERM' was not declared in this scope
   73 |      {CONTROL_C_EXIT,     SIGTERM}
      |                           ^~~~~~~

Once I add #include <signal.h>, it works again.

Am I the only one with this problem?


Regards
Hannes Domani

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Date: Mon, 6 Jan 2020 18:59:36 +0000 (UTC)
> From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>
> 
> C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c: In function 'int windows_status_to_termsig(long unsigned int)':
> C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c:56:37: error: 'SIGSEGV' was not declared in this scope
>    56 |      {EXCEPTION_ACCESS_VIOLATION,   SIGSEGV},
>       |                                     ^~~~~~~

Does the below help?

diff --git a/gdb/gdbsupport/gdb_wait.c b/gdb/gdbsupport/gdb_wait.c
index 037ba64..6facc48 100644
--- a/gdb/gdbsupport/gdb_wait.c
+++ b/gdb/gdbsupport/gdb_wait.c
@@ -34,7 +34,7 @@
    false positives is justified by the utility of reporting the
    terminating signal in the "normal" cases.  */
 
-# include "gdb/signals.h"	/* for enum gdb_signal */
+# include <signal.h>
 
 # define WIN32_LEAN_AND_MEAN
 # include <windows.h>		/* for EXCEPTION_* constants */

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Hannes Domani via gdb-patches]

Am Montag, 6. Januar 2020, 20:34:40 MEZ hat Eli Zaretskii <eliz@gnu.org> Folgendes geschrieben:

> > Date: Mon, 6 Jan 2020 18:59:36 +0000 (UTC)
> > From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>
> >
> > C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c: In function 'int windows_status_to_termsig(long unsigned int)':
> > C:/src/repos/binutils-gdb.git/gdb/gdbsupport/gdb_wait.c:56:37: error: 'SIGSEGV' was not declared in this scope
> >    56 |      {EXCEPTION_ACCESS_VIOLATION,   SIGSEGV},
> >       |                                     ^~~~~~~
>
> Does the below help?
>
> diff --git a/gdb/gdbsupport/gdb_wait.c b/gdb/gdbsupport/gdb_wait.c
> index 037ba64..6facc48 100644
> --- a/gdb/gdbsupport/gdb_wait.c
> +++ b/gdb/gdbsupport/gdb_wait.c
> @@ -34,7 +34,7 @@
>     false positives is justified by the utility of reporting the
>     terminating signal in the "normal" cases.  */
>
> -# include "gdb/signals.h"    /* for enum gdb_signal */
> +# include <signal.h>
>
>
> # define WIN32_LEAN_AND_MEAN
> # include <windows.h>        /* for EXCEPTION_* constants */

Yes, this works.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Date: Mon, 6 Jan 2020 19:38:50 +0000 (UTC)
> From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>
> 
> > Does the below help?
> >
> > diff --git a/gdb/gdbsupport/gdb_wait.c b/gdb/gdbsupport/gdb_wait.c
> > index 037ba64..6facc48 100644
> > --- a/gdb/gdbsupport/gdb_wait.c
> > +++ b/gdb/gdbsupport/gdb_wait.c
> > @@ -34,7 +34,7 @@
> >     false positives is justified by the utility of reporting the
> >     terminating signal in the "normal" cases.  */
> >
> > -# include "gdb/signals.h"    /* for enum gdb_signal */
> > +# include <signal.h>
> >
> >
> > # define WIN32_LEAN_AND_MEAN
> > # include <windows.h>        /* for EXCEPTION_* constants */
> 
> Yes, this works.

Thanks, pushed.

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Pedro Alves]

On 12/18/19 6:32 PM, Eli Zaretskii wrote:
>> Cc: philippe.waroquiers@skynet.be, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Wed, 18 Dec 2019 17:42:28 +0000
>>
>>> A new file for just one function sounds too much to me.  Is it OK to
>>> define an inline function in gdb_wait.h, as in the prototype change
>>> below?  If this way is accepted, I will post a fully formatted patch.
>>
>> I don't think it's too much.  As a static inline function means that
>> you end up with multiple versions of the function, including
>> the mapping array, in the gdb binary.  And also make gdb_wait.h
>> expose <windows.h>.  You could add a new gdbsupport/gdb_wait.c
>> file, with the function wrapped in #ifdef __MINGW32__ to keep it simple
>> and avoid host/target checks in the configure.ac files.
> 
> Sorry, I don't understand what host/target checks might be needed in
> configure.ac, can you explain?

What I meant is that files like mingw-hdep.o are conditionally added
to the set of files to build by configure.ac, depending on the host
triplet.  An alternative mechanism would be to merge all of 
mingw-hdep.c posix-hdep.c in a single .c file, and then use #ifdef
within.  I was suggesting the latter mechanism for this new file.


> 
>> We should not rely on signal.h constants for this hook.  See gdbarch.sh:
>>
>>  # Signal translation: translate the GDB's internal signal number into
>>  # the inferior's signal (target's) representation.  The implementation
>>  # of this method must be host independent.  IOW, don't rely on symbols
>>  # of the NAT_FILE header (the nm-*.h files), the host <signal.h>
>>  # header, or similar headers.
>>  # Return the target signal number if found, or -1 if the GDB internal
>>  # signal number is invalid.
>>  M;int;gdb_signal_to_target;enum gdb_signal signal;signal
>>
>> Say you're debugging against a mingw gdbserver from a Linux host.
>> If you rely on <signal.h> constants here, this function is going to
>> return the Linux (or whatever the host) numbers instead of the
>> Windows/mingw numbers.  For Linux, we define the LINUX_SIGxxx numbers
>> in linux-tdep.c, around line 125.  The patch should add a similar
>> enum.
> 
> So we need to have 2 different sets of explicit signal numbers, one
> for MinGW64, the other for mingw.org's MinGW (no, they are not
> identical)?  And perhaps one more for Cygwin?  Or should we just
> support the signals common to all 3 environments?
> 
> And what do we do when the signal numbers in the system's signal.h
> header change in some future version of MinGW/Cygwin?  

I would think signal numbers changing would be unlikely, since it
would be an ABI break.

> This sounds
> like a very fragile arrangement, at least for non-Posix systems where
> signals are emulated and aren't part of the kernel definitions set in
> stone.  I'm okay with doing a bunch of defines, but it seems to me a
> maintenance headache in the long run, FWIW.

The alternative is that the cross debugging scenario
doesn't work.  It doesn't seem better to me.  A potential alternative
would be to offload the mapping/conversions to the remote/server
side somehow, but that can't work for cross-core debugging, so it's
not ideal either.

> 
>> With this change, the user no longer has access to the original
>> $_exitcode, for the cases that match one of the known exceptions.
>> I don't know whether anyone is relying on those, though I wouldn't
>> be surprised if so.  I assume you've pondered about this and consider 
>> that the change is still worth it anyhow.  This should at least be
>> documented.  I wonder whether we should provide both the exit
>> code in $_exitcode and the translated signal number in $_exitsignal,
>> though that would require more changes.
> 
> I think this is a micro-optimization that is way in the area of the
> diminishing returns.  I've never seen a program to return an exit
> status with the high 2 bits set.  I'd definitely not recommend to
> tweak the current assumption in the GDB code that if a program exited
> due to a signal, its exit code is irrelevant, based on such a
> theoretical possibility.  In most cases, the fact that the inferior
> got a fatal exception will be noted before it exits anyway.

We're writing a debugger, and it doesn't seem odd to me that people
would like for the debugger to provide all information available
in order to debug the problem, rather than hide it.
You may be right that nobody actually cares about it, but I thought
I'd point it out so we can make an informed decision.

> 
>> Related, when windows_status_to_termsig doesn't recognize the
>> exception number, we end up with GDB_SIGNAL_UNKNOWN, and then
>> $_exitsignal == -1.  I.e., we lose information in that case.  Again,
>> something to ponder about that information loss is OK, or whether
>> we should do something about it.
> 
> I don't think we should do anything about it, it's highly theoretical
> situation I've never seen in real life.  And we already lose
> information in windows-nat.c, when we return GDB_SIGNAL_UNKNOWN for
> any exception we don't recognize explicitly.  

OK.  We do print the raw Win32 exception before converting to 
GDB_SIGNAL_UNKNOWN, so the user sees it, but I get your point.

> In any case, the way
> this stuff currently works, I see no simple way of returning an
> arbitrary value of a signal.

Yeah, I think we'd have to change struct target_waitstatus.

> 
> Maybe we should abandon the idea of doing this in windows-nat.c and
> win32-low.c, and only translate the exit code in cli-cmds.c for the
> 'pipe' command?  It's much more important in that case (since we don't
> intercept the signals as we do from the inferior), and most of those
> complications don't apply there.  There's also no backward
> compatibility problem, since 'pipe' is a new feature in GDB 9, with 2
> new convenience variables to hold the exit status and the signal
> value.  WDYT?

I don't know, I don't have a strong opinion.  As I mentioned, I'm
only pointing out the issues so we hash it all out and make an
informed decision.

I'll take a look at your new patch.

Thanks,
Pedro Alves

[RFAv3 4/6] Implement | (pipe) command.

[Philippe Waroquiers]

The pipe command allows to run a GDB command, and pipe its output
to a shell command:
  (gdb) help pipe
  Send the output of a gdb command to a shell command.
  Usage: pipe [COMMAND] | SHELL_COMMAND
  Usage: | [COMMAND] | SHELL_COMMAND
  Usage: pipe -dX COMMAND X SHELL_COMMAND
  Usage: | -dX COMMAND X SHELL_COMMAND
  Executes COMMAND and sends its output to SHELL_COMMAND.
  If COMMAND contains a | character, the option -dX indicates
  to use the character X to separate COMMAND from SHELL_COMMAND.
  With no COMMAND, repeat the last executed command
  and send its output to SHELL_COMMAND.
  (gdb)

For example:
  (gdb) pipe print some_data_structure | grep -B3 -A3 something

The pipe character is defined as an alias for pipe command, so that
the above can be typed as:
  (gdb) | print some_data_structure | grep -B3 -A3 something

If no GDB COMMAND is given, then the previous command is relaunched,
and its output is sent to the given SHELL_COMMAND.

This also defines convenience vars $_shell_exitcode and $_shell_exitsignal
to record the exit code and exit signal of the last shell command
launched by GDB e.g. by "shell", "pipe", ...

gdb/ChangeLog
2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-cmds.c (pipe_command): New function.
	(_initialize_cli_cmds): Call add_com for pipe_command.
	Define | as an alias for pipe.
	(exit_status_set_internal_vars): New function.
	(shell_escape): Call exit_status_set_internal_vars.
	cli/cli-decode.c (find_command_name_length): Recognize | as
	a single character command.
---
 gdb/cli/cli-cmds.c   | 105 +++++++++++++++++++++++++++++++++++++++++++
 gdb/cli/cli-decode.c |   4 +-
 2 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5f3b973f06..55fb5a9a7f 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -24,6 +24,7 @@
 #include "completer.h"
 #include "target.h"	/* For baud_rate, remote_debug and remote_timeout.  */
 #include "common/gdb_wait.h"	/* For shell escape implementation.  */
+#include "gdbcmd.h"
 #include "gdb_regex.h"	/* Used by apropos_command.  */
 #include "gdb_vfork.h"
 #include "linespec.h"
@@ -41,6 +42,7 @@
 #include "block.h"
 
 #include "ui-out.h"
+#include "interps.h"
 
 #include "top.h"
 #include "cli/cli-decode.h"
@@ -695,6 +697,25 @@ echo_command (const char *text, int from_tty)
   gdb_flush (gdb_stdout);
 }
 
+/* Sets the last launched shell command convenience variables based on
+   EXIT_STATUS.  */
+
+static void
+exit_status_set_internal_vars (int exit_status)
+{
+  struct internalvar *var_code = lookup_internalvar ("_shell_exitcode");
+  static internalvar *var_signal = lookup_internalvar ("_shell_exitsignal");
+
+  clear_internalvar (var_code);
+  clear_internalvar (var_signal);
+  if (WIFEXITED (exit_status))
+    set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
+  else if (WIFSIGNALED (exit_status))
+    set_internalvar_integer (var_signal, WTERMSIG (exit_status));
+  else
+    warning (_("unexpected shell command exit_status %d\n"), exit_status);
+}
+
 static void
 shell_escape (const char *arg, int from_tty)
 {
@@ -716,6 +737,7 @@ shell_escape (const char *arg, int from_tty)
   /* Make sure to return to the directory GDB thinks it is, in case
      the shell command we just ran changed it.  */
   chdir (current_directory);
+  exit_status_set_internal_vars (rc);
 #endif
 #else /* Can fork.  */
   int status, pid;
@@ -743,6 +765,7 @@ shell_escape (const char *arg, int from_tty)
     waitpid (pid, &status, 0);
   else
     error (_("Fork failed"));
+  exit_status_set_internal_vars (status);
 #endif /* Can fork.  */
 }
 
@@ -854,6 +877,75 @@ edit_command (const char *arg, int from_tty)
   xfree (p);
 }
 
+/* Implementation of the "pipe" command.  */
+
+static void
+pipe_command (const char *arg, int from_tty)
+{
+  const char *command = arg;
+  const char *shell_command = arg;
+  std::string separator ("|");
+
+  if (arg == NULL)
+    error (_("Missing COMMAND"));
+
+  shell_command = skip_spaces (shell_command);
+
+  if (*shell_command == '-' && *(shell_command + 1) == 'd')
+    {
+      shell_command += 2; /* Skip '-d'.  */
+      separator = extract_arg (&shell_command);
+      if (separator.empty ())
+	error (_("Missing separator SEP after -d"));
+      command = shell_command;
+    }
+
+  shell_command = strstr (shell_command, separator.c_str ());
+
+  if (shell_command == nullptr)
+    error (_("Missing separator before SHELL_COMMAND"));
+
+  command = skip_spaces (command);
+  std::string gdb_cmd (command, shell_command - command);
+
+  if (gdb_cmd.empty ())
+    {
+      repeat_previous ();
+      gdb_cmd = skip_spaces (get_saved_command_line ());
+      if (gdb_cmd.empty ())
+	error (_("No previous command to relaunch"));
+    }
+
+  shell_command += separator.length (); /* Skip the separator.  */
+  shell_command = skip_spaces (shell_command);
+  if (*shell_command == '\0')
+    error (_("Missing SHELL_COMMAND"));
+
+  FILE *to_shell_command = popen (shell_command, "w");
+
+  if (to_shell_command == nullptr)
+    error (_("Error launching \"%s\""), shell_command);
+
+  try
+    {
+      stdio_file pipe_file (to_shell_command);
+
+      execute_command_to_ui_file (&pipe_file, gdb_cmd.c_str (), from_tty);
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      pclose (to_shell_command);
+      throw;
+    }
+
+  int exit_status = pclose (to_shell_command);
+
+  if (exit_status < 0)
+    error (_("shell command  \"%s\" errno %s"), shell_command,
+           safe_strerror (errno));
+  exit_status_set_internal_vars (exit_status);
+}
+
 static void
 list_command (const char *arg, int from_tty)
 {
@@ -1845,6 +1937,19 @@ Uses EDITOR environment variable contents as editor (or ex as default)."));
 
   c->completer = location_completer;
 
+  c = add_com ("pipe", class_support, pipe_command, _("\
+Send the output of a gdb command to a shell command.\n\
+Usage: | [COMMAND] | SHELL_COMMAND\n\
+Usage: | -d SEP COMMAND SEP SHELL_COMMAND\n\
+Usage: pipe [COMMAND] | SHELL_COMMAND\n\
+Usage: pipe -d SEP COMMAND SEP SHELL_COMMAND\n\
+Executes COMMAND and sends its output to SHELL_COMMAND.\n\
+If COMMAND contains a | character, the option -d SEP indicates\n\
+to use the string SEP to separate COMMAND from SHELL_COMMAND.\n\
+With no COMMAND, repeat the last executed command\n\
+and send its output to SHELL_COMMAND."));
+  add_com_alias ("|", "pipe", class_support, 0);
+
   add_com ("list", class_files, list_command, _("\
 List specified function or line.\n\
 With no argument, lists ten more lines after or around previous listing.\n\
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 50430953c7..46051090cd 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1311,9 +1311,9 @@ find_command_name_length (const char *text)
      Note that this is larger than the character set allowed when
      creating user-defined commands.  */
 
-  /* Recognize '!' as a single character command so that, e.g., "!ls"
+  /* Recognize the single character commands so that, e.g., "!ls"
      works as expected.  */
-  if (*p == '!')
+  if (*p == '!' || *p == '|')
     return 1;
 
   while (isalnum (*p) || *p == '-' || *p == '_'
-- 
2.20.1

Re: [RFAv3 4/6] Implement | (pipe) command.

[Pedro Alves]

On 5/4/19 5:17 PM, Philippe Waroquiers wrote:
> The pipe command allows to run a GDB command, and pipe its output
> to a shell command:
>   (gdb) help pipe
>   Send the output of a gdb command to a shell command.
>   Usage: pipe [COMMAND] | SHELL_COMMAND
>   Usage: | [COMMAND] | SHELL_COMMAND
>   Usage: pipe -dX COMMAND X SHELL_COMMAND
>   Usage: | -dX COMMAND X SHELL_COMMAND
>   Executes COMMAND and sends its output to SHELL_COMMAND.
>   If COMMAND contains a | character, the option -dX indicates
>   to use the character X to separate COMMAND from SHELL_COMMAND.
>   With no COMMAND, repeat the last executed command
>   and send its output to SHELL_COMMAND.
>   (gdb)

This help output is stale now.  The current output is:

(gdb) help pipe 
Send the output of a gdb command to a shell command.
Usage: | [COMMAND] | SHELL_COMMAND
Usage: | -d SEP COMMAND SEP SHELL_COMMAND
Usage: pipe [COMMAND] | SHELL_COMMAND
Usage: pipe -d SEP COMMAND SEP SHELL_COMMAND
Executes COMMAND and sends its output to SHELL_COMMAND.
If COMMAND contains a | character, the option -d SEP indicates
to use the string SEP to separate COMMAND from SHELL_COMMAND.
With no COMMAND, repeat the last executed command
and send its output to SHELL_COMMAND.
(gdb) 

I think the "If COMMAND contains a | character," could be
improved with a bit of copy/editing.  Something like this:

(gdb) help pipe 
Send the output of a gdb command to a shell command.
Usage: | [COMMAND] | SHELL_COMMAND
Usage: | -d SEP COMMAND SEP SHELL_COMMAND
Usage: pipe [COMMAND] | SHELL_COMMAND
Usage: pipe -d SEP COMMAND SEP SHELL_COMMAND

Executes COMMAND and sends its output to SHELL_COMMAND.

The -d option indicates to use the string SEP to separate COMMAND
from SHELL_COMMAND, in alternative to |.  This is useful in
case COMMAND contains a | character.

With no COMMAND, repeat the last executed command
and send its output to SHELL_COMMAND.


(Or see the suggestion in my reply to the manual patch).


Also, is there a reason you picked "-d" for the option
letter?  Maybe you were thinking of "delimiter"?  In
that case, maybe consider describing it with
"-d DEL" or "-d DELIM" instead of "-d SEP", but better
mnemonics.  Just a suggestion.

> For example:
>   (gdb) pipe print some_data_structure | grep -B3 -A3 something
> 
> The pipe character is defined as an alias for pipe command, so that
> the above can be typed as:
>   (gdb) | print some_data_structure | grep -B3 -A3 something
> 
> If no GDB COMMAND is given, then the previous command is relaunched,
> and its output is sent to the given SHELL_COMMAND.
> 
> This also defines convenience vars $_shell_exitcode and $_shell_exitsignal
> to record the exit code and exit signal of the last shell command
> launched by GDB e.g. by "shell", "pipe", ...
> 
> gdb/ChangeLog
> 2019-05-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* cli/cli-cmds.c (pipe_command): New function.
> 	(_initialize_cli_cmds): Call add_com for pipe_command.
> 	Define | as an alias for pipe.
> 	(exit_status_set_internal_vars): New function.
> 	(shell_escape): Call exit_status_set_internal_vars.
> 	cli/cli-decode.c (find_command_name_length): Recognize | as
> 	a single character command.
> ---
>  gdb/cli/cli-cmds.c   | 105 +++++++++++++++++++++++++++++++++++++++++++
>  gdb/cli/cli-decode.c |   4 +-
>  2 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 5f3b973f06..55fb5a9a7f 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -24,6 +24,7 @@
>  #include "completer.h"
>  #include "target.h"	/* For baud_rate, remote_debug and remote_timeout.  */
>  #include "common/gdb_wait.h"	/* For shell escape implementation.  */
> +#include "gdbcmd.h"
>  #include "gdb_regex.h"	/* Used by apropos_command.  */
>  #include "gdb_vfork.h"
>  #include "linespec.h"
> @@ -41,6 +42,7 @@
>  #include "block.h"
>  
>  #include "ui-out.h"
> +#include "interps.h"
>  
>  #include "top.h"
>  #include "cli/cli-decode.h"
> @@ -695,6 +697,25 @@ echo_command (const char *text, int from_tty)
>    gdb_flush (gdb_stdout);
>  }
>  
> +/* Sets the last launched shell command convenience variables based on
> +   EXIT_STATUS.  */
> +
> +static void
> +exit_status_set_internal_vars (int exit_status)
> +{
> +  struct internalvar *var_code = lookup_internalvar ("_shell_exitcode");
> +  static internalvar *var_signal = lookup_internalvar ("_shell_exitsignal");
     ^^^^^^

Did you really mean static ?  At least, did you mean it just for one of them?

> +
> +  clear_internalvar (var_code);
> +  clear_internalvar (var_signal);
> +  if (WIFEXITED (exit_status))
> +    set_internalvar_integer (var_code, WEXITSTATUS (exit_status));
> +  else if (WIFSIGNALED (exit_status))
> +    set_internalvar_integer (var_signal, WTERMSIG (exit_status));
> +  else
> +    warning (_("unexpected shell command exit_status %d\n"), exit_status);

That "exit_status" in the warning is leaking out the internal variable
name in a user-facing message.

> +}
> +
>  static void
>  shell_escape (const char *arg, int from_tty)
>  {
> @@ -716,6 +737,7 @@ shell_escape (const char *arg, int from_tty)
>    /* Make sure to return to the directory GDB thinks it is, in case
>       the shell command we just ran changed it.  */
>    chdir (current_directory);
> +  exit_status_set_internal_vars (rc);
>  #endif
>  #else /* Can fork.  */
>    int status, pid;
> @@ -743,6 +765,7 @@ shell_escape (const char *arg, int from_tty)
>      waitpid (pid, &status, 0);
>    else
>      error (_("Fork failed"));
> +  exit_status_set_internal_vars (status);
>  #endif /* Can fork.  */
>  }
>  
> @@ -854,6 +877,75 @@ edit_command (const char *arg, int from_tty)
>    xfree (p);
>  }
>  
> +/* Implementation of the "pipe" command.  */
> +
> +static void
> +pipe_command (const char *arg, int from_tty)
> +{
> +  const char *command = arg;
> +  const char *shell_command = arg;
> +  std::string separator ("|");
> +
> +  if (arg == NULL)
> +    error (_("Missing COMMAND"));
> +
> +  shell_command = skip_spaces (shell_command);

This isn't necessary.

> +
> +  if (*shell_command == '-' && *(shell_command + 1) == 'd')
> +    {
> +      shell_command += 2; /* Skip '-d'.  */

Use check_for_argument.

> +      separator = extract_arg (&shell_command);
> +      if (separator.empty ())
> +	error (_("Missing separator SEP after -d"));
> +      command = shell_command;
> +    }
> +
> +  shell_command = strstr (shell_command, separator.c_str ());
> +
> +  if (shell_command == nullptr)

Let's be consistent with NULL vs nullptr in new code.

> +    error (_("Missing separator before SHELL_COMMAND"));


BTW, I suspect the section above would look clearer if it
referred to arg instead of shell_command, and declared
command/shell_command where they're found.  Like:

  if (check_for_argument (arg, "-d", 2))
    {
      separator = extract_arg (&arg);
      if (separator.empty ())
	error (_("Missing separator SEP after -d"));
    }

  arg = skip_spaces (arg);

  const char *command = arg;

  const char *shell_command = strstr (arg, separator.c_str ());

  if (shell_command == nullptr)
    error (_("Missing separator before SHELL_COMMAND"));

> +
> +  command = skip_spaces (command);
> +  std::string gdb_cmd (command, shell_command - command);
> +
> +  if (gdb_cmd.empty ())
> +    {
> +      repeat_previous ();
> +      gdb_cmd = skip_spaces (get_saved_command_line ());
> +      if (gdb_cmd.empty ())
> +	error (_("No previous command to relaunch"));
> +    }
> +
> +  shell_command += separator.length (); /* Skip the separator.  */
> +  shell_command = skip_spaces (shell_command);
> +  if (*shell_command == '\0')
> +    error (_("Missing SHELL_COMMAND"));
> +
> +  FILE *to_shell_command = popen (shell_command, "w");
> +
> +  if (to_shell_command == nullptr)
> +    error (_("Error launching \"%s\""), shell_command);
> +
> +  try
> +    {
> +      stdio_file pipe_file (to_shell_command);

stdio_file's destructor calls fclose unless you tell it otherwise:

  /* Create a ui_file from a previously opened FILE.  CLOSE_P
     indicates whether the underlying file should be closed when the
     stdio_file is destroyed.  */
  explicit stdio_file (FILE *file, bool close_p = false);

> +
> +      execute_command_to_ui_file (&pipe_file, gdb_cmd.c_str (), from_tty);

So here this is calling fclose before leaving the scope.  But popen
FILE's should be closed with pclose, only.  I don't know why it isn't
causing problems, may be the glibc's fclose does nothing in this case.
We shouldn't rely on that.

> +    }
> +  catch (const gdb_exception_error &ex)

This should catch all kinds of exceptions, not just errors.  E.g.,
Ctrl-C results in gdb_exception_quit, which this doesn't catch.

Write, literally:

    catch (...)

to be clear that we want to catch everything.

> +    {
> +      pclose (to_shell_command);
> +      throw;
> +    }
> +
> +  int exit_status = pclose (to_shell_command);
> +
> +  if (exit_status < 0)
> +    error (_("shell command  \"%s\" errno %s"), shell_command,
> +           safe_strerror (errno));

Spurious double space in the error message.  Note you say "errno",
but this isn't printing the errno (the error number).  I'd
suggest printing in perror_with_name style:

    error (_("shell command \"%s\" failed: %s"), shell_command,
           safe_strerror (errno));

Thanks,
Pedro Alves

Re: [RFAv3 4/6] Implement | (pipe) command.

[Pedro Alves]

On 5/27/19 6:48 PM, Pedro Alves wrote:
> stdio_file's destructor calls fclose unless you tell it otherwise:
> 
>   /* Create a ui_file from a previously opened FILE.  CLOSE_P
>      indicates whether the underlying file should be closed when the
>      stdio_file is destroyed.  */
>   explicit stdio_file (FILE *file, bool close_p = false);
> 
>> +
>> +      execute_command_to_ui_file (&pipe_file, gdb_cmd.c_str (), from_tty);
> So here this is calling fclose before leaving the scope.  But popen
> FILE's should be closed with pclose, only.  I don't know why it isn't
> causing problems, may be the glibc's fclose does nothing in this case.
> We shouldn't rely on that.
> 

Ahahaha.  The argument defaults to false, not true.  Silly me.

Thanks,
Pedro Alves

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Hannes Domani via gdb-patches]

 While locally fixing a merge-conflict I noticed the following:

Am Freitag, 3. Januar 2020, 20:59:45 MEZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben:

> Here's the updated patch.  I confirmed that it builds on MinGW-W64
> using my Fedora's cross cross compiler, but I didn't try to run
> the resulting GDB (since I'm not on Windows).

> diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> index ca9b81df298..58f8838b885 100644
> --- a/gdb/windows-tdep.c
> +++ b/gdb/windows-tdep.c
> @@ -35,6 +35,57 @@
> #include "solib-target.h"
> #include "gdbcore.h"
>
> +/* Windows signal numbers differ between MinGW flavors and between
> +  those and Cygwin.  The below enumeration was gleaned from the
> +  respective headers; the ones marked with MinGW64/Cygwin are defined
> +  only by MinGW64 and Cygwin, not by mingw.org's MinGW.  FIXME: We
> +  should really have distinct MinGW vs Cygwin OSABIs, and two
> +  separate enums, selected at runtime.  */
> +
> +enum
> +  {
> +  WINDOWS_SIGHUP = 1,    /* MinGW64/Cygwin */
> +  WINDOWS_SIGINT = 2,
> +  WINDOWS_SIGQUIT = 3,    /* MinGW64/Cygwin */
> +  WINDOWS_SIGILL = 4,
> +  WINDOWS_SIGTRAP = 5,    /* MinGW64/Cygwin */
> +#ifdef __CYGWIN__
> +  WINDOWS_SGABRT = 6,
> +#else
> +  WINDOWS_SIGIOT = 6,    /* MinGW64 */
> +#endif
> +  WINDOWS_SIGEMT = 7,    /* MinGW64/Cygwin */
> +  WINDOWS_SIGFPE = 8,
> +  WINDOWS_SIGKILL = 9,    /* MinGW64/Cygwin */
> +  WINDOWS_SIGBUS = 10,    /* MinGW64/Cygwin */
> +  WINDOWS_SIGSEGV = 11,
> +  WINDOWS_SIGSYS = 12,    /* MinGW64/Cygwin */
> +  WINDOWS_SIGPIPE = 13,/* MinGW64/Cygwin */
> +  WINDOWS_SIGALRM = 14,/* MinGW64/Cygwin */
> +  WINDOWS_SIGTERM = 15,
> +#ifdef __CYGWIN__
> +  WINDOWS_SIGURG = 16,
> +  WINDOWS_SIGSTOP = 17,
> +  WINDOWS_SIGTSTP = 18,
> +  WINDOWS_SIGCONT = 19,
> +  WINDOWS_SIGCHLD = 20,
> +  WINDOWS_SIGTTIN = 21,
> +  WINDOWS_SIGTTOU = 22,
> +  WINDOWS_SIGIO = 23,
> +  WINDOWS_SIGXCPU = 24,
> +  WINDOWS_SIGXFSZ = 25,
> +  WINDOWS_SIGVTALRM = 26,
> +  WINDOWS_SIGPROF = 27,
> +  WINDOWS_SIGWINCH = 28,
> +  WINDOWS_SIGLOST = 29,
> +  WINDOWS_SIGUSR1 = 30,
> +  WINDOWS_SIGUSR2 = 31
> +#else
> +  WINDOWS_SIGBREAK = 21,
> +  WINDOWS_SIGABRT = 22
> +#endif
> +  };

For __CYGWIN__ it's WINDOWS_SGABRT, otherwise it's WINDOWS_SIGABRT.

> +
> struct cmd_list_element *info_w32_cmdlist;
>
> typedef struct thread_information_block_32
> @@ -461,6 +512,83 @@ init_w32_command_list (void)
>     }
> }
>
> +/* Implementation of `gdbarch_gdb_signal_to_target'.  */
> +
> +static int
> +windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
> +{
> +  switch (signal)
> +    {
> +    case GDB_SIGNAL_0:
> +      return 0;
> +    case GDB_SIGNAL_HUP:
> +      return WINDOWS_SIGHUP;
> +    case GDB_SIGNAL_INT:
> +      return WINDOWS_SIGINT;
> +    case GDB_SIGNAL_QUIT:
> +      return WINDOWS_SIGQUIT;
> +    case GDB_SIGNAL_ILL:
> +      return WINDOWS_SIGILL;
> +    case GDB_SIGNAL_TRAP:
> +      return WINDOWS_SIGTRAP;
> +    case GDB_SIGNAL_ABRT:
> +      return WINDOWS_SIGABRT;

I don't think this compiles for cygwin.

> +    case GDB_SIGNAL_EMT:
> +      return WINDOWS_SIGEMT;
> +    case GDB_SIGNAL_FPE:
> +      return WINDOWS_SIGFPE;
> +    case GDB_SIGNAL_KILL:
> +      return WINDOWS_SIGKILL;
> +    case GDB_SIGNAL_BUS:
> +      return WINDOWS_SIGBUS;
> +    case GDB_SIGNAL_SEGV:
> +      return WINDOWS_SIGSEGV;
> +    case GDB_SIGNAL_SYS:
> +      return WINDOWS_SIGSYS;
> +    case GDB_SIGNAL_PIPE:
> +      return WINDOWS_SIGPIPE;
> +    case GDB_SIGNAL_ALRM:
> +      return WINDOWS_SIGALRM;
> +    case GDB_SIGNAL_TERM:
> +      return WINDOWS_SIGTERM;
> +#ifdef __CYGWIN__
> +    case GDB_SIGNAL_URG:
> +      return WINDOWS_SIGURG;
> +    case GDB_SIGNAL_STOP:
> +      return WINDOWS_SIGSTOP;
> +    case GDB_SIGNAL_TSTP:
> +      return WINDOWS_SIGTSTP;
> +    case GDB_SIGNAL_CONT:
> +      return WINDOWS_SIGCONT;
> +    case GDB_SIGNAL_CHLD:
> +      return WINDOWS_SIGCHLD;
> +    case GDB_SIGNAL_TTIN:
> +      return WINDOWS_SIGTTIN;
> +    case GDB_SIGNAL_TTOU:
> +      return WINDOWS_SIGTTOU;
> +    case GDB_SIGNAL_IO:
> +      return WINDOWS_SIGIO;
> +    case GDB_SIGNAL_XCPU:
> +      return WINDOWS_SIGXCPU;
> +    case GDB_SIGNAL_XFSZ:
> +      return WINDOWS_SIGXFSZ;
> +    case GDB_SIGNAL_VTALRM:
> +      return WINDOWS_SIGVTALRM;
> +    case GDB_SIGNAL_PROF:
> +      return WINDOWS_SIGPROF;
> +    case GDB_SIGNAL_WINCH:
> +      return WINDOWS_SIGWINCH;
> +    case GDB_SIGNAL_PWR:
> +      return WINDOWS_SIGLOST;
> +    case GDB_SIGNAL_USR1:
> +      return WINDOWS_SIGUSR1;
> +    case GDB_SIGNAL_USR2:
> +      return WINDOWS_SIGUSR2;
> +#endif    /* __CYGWIN__ */
> +    }
> +  return -1;
> +}
> +
> /* To be called from the various GDB_OSABI_CYGWIN handlers for the
>     various Windows architectures and machine types.  */
>
> @@ -477,6 +605,8 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>   set_gdbarch_iterate_over_objfiles_in_search_order
>     (gdbarch, windows_iterate_over_objfiles_in_search_order);
>
> +  set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target);
> +
>   set_solib_ops (gdbarch, &solib_target_so_ops);
> }
>
>
> base-commit: 44f81a76542dbeada2541a05de191ae0ac0fbc2c
> --
> 2.14.5

Re: [RFAv3 2/6] Improve process exit status macros on MinGW

[Eli Zaretskii]

> Date: Mon, 6 Jan 2020 17:47:06 +0000 (UTC)
> From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>
> 
>  While locally fixing a merge-conflict I noticed the following:

Thanks, pushed the below as obvious.

commit 89a65580f4522f81ef7e4e49298b24f3ebc14355
Author:     Eli Zaretskii <eliz@gnu.org>
AuthorDate: Mon Jan 6 20:22:15 2020 +0200
Commit:     Eli Zaretskii <eliz@gnu.org>
CommitDate: Mon Jan 6 20:22:15 2020 +0200

    Fix a typo in gdb/windows-tdep.c
    
    gdb/ChangeLog
    2020-01-06  Eli Zaretskii  <eliz@gnu.org>
    
                * windows-tdep.c: Fix a typo in WINDOWS_SIGABRT.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ad7c33f..177b505 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-01-06  Eli Zaretskii  <eliz@gnu.org>
+
+	* windows-tdep.c: Fix a typo in WINDOWS_SIGABRT.
+
 2020-01-06  Hannes Domani  <ssbssa@yahoo.de>
 
 	* source.c (print_source_lines_base): Set last_line_listed.
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index 58f8838..b6e5b9f 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -50,7 +50,7 @@ enum
    WINDOWS_SIGILL = 4,
    WINDOWS_SIGTRAP = 5,	/* MinGW64/Cygwin */
 #ifdef __CYGWIN__
-   WINDOWS_SGABRT = 6,
+   WINDOWS_SIGABRT = 6,
 #else
    WINDOWS_SIGIOT = 6,	/* MinGW64 */
 #endif