| Message ID | 20190504161753.15530-3-philippe.waroquiers@skynet.be |
|---|---|
| State | New |
| Headers | show |
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
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> {