This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]