[RFAv3,2/6] Improve process exit status macros on MinGW
Commit Message
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(-)
Comments
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
> 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.
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
> 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.
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
> 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
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
@@ -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
@@ -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>
{