[4/5] Fix exit/signal code on Cygwin

Message ID 20260522001626.393908-5-pedro@palves.net
State New
Headers
Series Fix a few Cygwin/MinGW problems |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Pedro Alves May 22, 2026, 12:16 a.m. UTC
  I noticed that on native Cygwin, gdb.python/py-events.exp has this:

 [Thread 15952.0x534 (id 1) exited with code 12]
...
 Program terminated with signal SIGSYS, Bad system call.
 ...
 (gdb) FAIL: gdb.python/py-events.exp: Inferior 1 terminated.

The program exits with normal exit code 12, not a signal.  SIGSYS is
12.

Similarly, gdb.base/exitsignal.exp has this:

 continue
 Continuing.
 [Thread 15220.0x219c (id 1) exited with code 2816]
 [Thread 15220.0x3a50 (id 3) exited with code 2816]
 [Thread 15220.0x25a0 (id 4) exited with code 2816]
 [Inferior 1 (process 15220) exited with code 05400]
 (gdb) FAIL: gdb.base/exitsignal.exp: program terminated with SIGSEGV (the program exited)

Here, the program exits with SIGSEGV, not normal exit code 2816 (05400
in octal).

The problem is that gdb/windows-nat.c does not know about Cygwin's
exit codes as seen from the native Windows side.  Same for gdbserver's
win32-low.c.

This commit fixes it.  To avoid duplicating code, it adds a new
native_exit_code_to_target_status function in nat/windows-nat.c used
by both GDB and GDBserver, with the MinGW-specific logic added by
commit 559e7e5056 ("Improve process exit status macros on MinGW")
moved there too.

Change-Id: I5c4d9cd81209d46598575518ef2fd205d77f9b66
commit-id: 153617c2
---
 gdb/nat/windows-nat.c  | 42 ++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/windows-nat.h  |  4 ++++
 gdb/windows-nat.c      | 14 ++------------
 gdbserver/win32-low.cc | 13 ++-----------
 4 files changed, 50 insertions(+), 23 deletions(-)
  

Comments

Eli Zaretskii May 22, 2026, 7:15 a.m. UTC | #1
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 22 May 2026 01:16:25 +0100
> 
> I noticed that on native Cygwin, gdb.python/py-events.exp has this:
> 
>  [Thread 15952.0x534 (id 1) exited with code 12]
> ...
>  Program terminated with signal SIGSYS, Bad system call.
>  ...
>  (gdb) FAIL: gdb.python/py-events.exp: Inferior 1 terminated.
> 
> The program exits with normal exit code 12, not a signal.  SIGSYS is
> 12.
> 
> Similarly, gdb.base/exitsignal.exp has this:
> 
>  continue
>  Continuing.
>  [Thread 15220.0x219c (id 1) exited with code 2816]
>  [Thread 15220.0x3a50 (id 3) exited with code 2816]
>  [Thread 15220.0x25a0 (id 4) exited with code 2816]
>  [Inferior 1 (process 15220) exited with code 05400]
>  (gdb) FAIL: gdb.base/exitsignal.exp: program terminated with SIGSEGV (the program exited)
> 
> Here, the program exits with SIGSEGV, not normal exit code 2816 (05400
> in octal).
> 
> The problem is that gdb/windows-nat.c does not know about Cygwin's
> exit codes as seen from the native Windows side.  Same for gdbserver's
> win32-low.c.
> 
> This commit fixes it.  To avoid duplicating code, it adds a new
> native_exit_code_to_target_status function in nat/windows-nat.c used
> by both GDB and GDBserver, with the MinGW-specific logic added by
> commit 559e7e5056 ("Improve process exit status macros on MinGW")
> moved there too.

AFAIU, this basically adds a Cygwin-specific branch to the code that
determines the terminating signal and status of a program, leaving the
code for the native Windows and MinGW programs intact.  I suggest to
say this in the commit log message, because as written, it sounds like
it does something for Cygwin that is not done for MinGW.  Which is not
true.

> +#ifdef __CYGWIN__
> +  /* /usr/include/cygwin/wait.h explains that a wait status is 16
> +     bits, and looks like:
> +
> +       "<1 byte info> <1 byte code>
> +       <code> == 0, child has exited, info is the exit value
> +       <code> == 1..7e, child has exited, code is the signal number.
> +       <code> == 7f, child has stopped, info was the signal number.
> +       <code> == 80, there was a core dump."
> +
> +     However, when passing the wait status to native ExitProcess as a
> +     native exit code, cygwin1.dll swaps the <info>/<code> bytes.
> +     Swap them back into a wait status here.  */
> +  int wstatus = ((exit_code & 0xff) << 8) | ((exit_code >> 8) & 0xff);

Should the commentary say something about _why_ we swap the bytes
here?  I know very little about Cygwin, but my naïve view is that when
a Cygwin program is run from another Cygwin program, no such swapping
should be needed, is that right?  One should just use the macros from
the sys/wait.h header, right?  If so, why do we need to swap the bytes
here, and how is "passing the wait status to native ExitProcess as a
native exit code" relevant to what this part of GDB needs to do?

Other than these questions, the MinGW part of the code looks okay to
me, thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  
Pedro Alves May 25, 2026, 4:47 p.m. UTC | #2
On 2026-05-22 08:15, Eli Zaretskii wrote:
>> From: Pedro Alves <pedro@palves.net>

...

>> This commit fixes it.  To avoid duplicating code, it adds a new
>> native_exit_code_to_target_status function in nat/windows-nat.c used
>> by both GDB and GDBserver, with the MinGW-specific logic added by
>> commit 559e7e5056 ("Improve process exit status macros on MinGW")
>> moved there too.
> 
> AFAIU, this basically adds a Cygwin-specific branch to the code that
> determines the terminating signal and status of a program, leaving the
> code for the native Windows and MinGW programs intact.  

Yes.  For v2, I split the refactor to its own patch, which I think makes
that more obvious.

> I suggest to
> say this in the commit log message, because as written, it sounds like
> it does something for Cygwin that is not done for MinGW.  Which is not
> true.

Done this too, thanks.

> 
>> +#ifdef __CYGWIN__
>> +  /* /usr/include/cygwin/wait.h explains that a wait status is 16
>> +     bits, and looks like:
>> +
>> +       "<1 byte info> <1 byte code>
>> +       <code> == 0, child has exited, info is the exit value
>> +       <code> == 1..7e, child has exited, code is the signal number.
>> +       <code> == 7f, child has stopped, info was the signal number.
>> +       <code> == 80, there was a core dump."
>> +
>> +     However, when passing the wait status to native ExitProcess as a
>> +     native exit code, cygwin1.dll swaps the <info>/<code> bytes.
>> +     Swap them back into a wait status here.  */
>> +  int wstatus = ((exit_code & 0xff) << 8) | ((exit_code >> 8) & 0xff);
> 
> Should the commentary say something about _why_ we swap the bytes
> here?  I know very little about Cygwin, but my naïve view is that when
> a Cygwin program is run from another Cygwin program, no such swapping
> should be needed, is that right?  One should just use the macros from
> the sys/wait.h header, right?  If so, why do we need to swap the bytes
> here, and how is "passing the wait status to native ExitProcess as a
> native exit code" relevant to what this part of GDB needs to do?

Thanks for raising the question, it made me look deeper, dig into the Cygwin code
in mode detail, and realize that in the attach case, we shouldn't do the swap.
v2 has more and better comments explaining all this, as well as handling the
attach case, and testing it too.  The new comments will implicitly answer your
questions.  I'll be sending v2 shortly to the list.

Pedro Alves

> 
> Other than these questions, the MinGW part of the code looks okay to
> me, thanks.
> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
>
  

Patch

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index b093acda342..4c8c9ea32a8 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -18,6 +18,8 @@ 
 
 #include "nat/windows-nat.h"
 #include "gdbsupport/common-debug.h"
+#include "gdbsupport/gdb_signals.h"
+#include "gdbsupport/gdb_wait.h"
 #include "target/target.h"
 
 #undef GetModuleFileNameEx
@@ -694,6 +696,46 @@  windows_process_info::add_all_dlls ()
 
 /* See nat/windows-nat.h.  */
 
+target_waitstatus
+native_exit_code_to_target_status (DWORD exit_code)
+{
+  target_waitstatus tstatus;
+
+#ifdef __CYGWIN__
+  /* /usr/include/cygwin/wait.h explains that a wait status is 16
+     bits, and looks like:
+
+       "<1 byte info> <1 byte code>
+       <code> == 0, child has exited, info is the exit value
+       <code> == 1..7e, child has exited, code is the signal number.
+       <code> == 7f, child has stopped, info was the signal number.
+       <code> == 80, there was a core dump."
+
+     However, when passing the wait status to native ExitProcess as a
+     native exit code, cygwin1.dll swaps the <info>/<code> bytes.
+     Swap them back into a wait status here.  */
+  int wstatus = ((exit_code & 0xff) << 8) | ((exit_code >> 8) & 0xff);
+  if (!WIFSIGNALED (wstatus))
+    tstatus.set_exited (WEXITSTATUS (wstatus));
+  else
+    tstatus.set_signalled (gdb_signal_from_host (WTERMSIG (wstatus)));
+#else
+  /* 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_code) ? WTERMSIG (exit_code) : -1;
+  if (exit_signal == -1)
+    tstatus.set_exited (exit_code);
+  else
+    tstatus.set_signalled (gdb_signal_from_host (exit_signal));
+#endif
+
+  return tstatus;
+}
+
+/* See nat/windows-nat.h.  */
+
 std::string
 event_code_to_string (DWORD event_code)
 {
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 52378765438..1cabe288cee 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -347,6 +347,10 @@  struct windows_process_info
   int get_exec_module_filename (char *exe_name_ret, size_t exe_name_max_len);
 };
 
+/* Convert a native ExitProcess exit code to a target wait status.  */
+
+extern target_waitstatus native_exit_code_to_target_status (DWORD exit_code);
+
 /* Return a string version of EVENT_CODE.  */
 
 extern std::string event_code_to_string (DWORD event_code);
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index a284438bd36..54755a4c996 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -62,7 +62,6 @@ 
 #include "complaints.h"
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/pathstuff.h"
-#include "gdbsupport/gdb_wait.h"
 #include "gdbsupport/symbol.h"
 #include "inf-loop.h"
 
@@ -1610,17 +1609,8 @@  windows_nat_target::get_windows_debug_event
 	}
       else if (windows_process->saw_create == 1)
 	{
-	  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->set_exited (exit_status);
-	  else
-	    ourstatus->set_signalled (gdb_signal_from_host (exit_signal));
+	  DWORD exit_code = current_event->u.ExitProcess.dwExitCode;
+	  *ourstatus = native_exit_code_to_target_status (exit_code);
 
 	  thread_id = current_event->dwThreadId;
 
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 6f1cf5ed025..23812c0689f 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -33,7 +33,6 @@ 
 #include <process.h>
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/common-inferior.h"
-#include "gdbsupport/gdb_wait.h"
 
 using namespace windows_nat;
 
@@ -1062,16 +1061,8 @@  get_child_debug_event (DWORD *continue_status,
 
     case EXIT_PROCESS_DEBUG_EVENT:
       {
-	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->set_exited (exit_status);
-	else
-	  ourstatus->set_signalled (gdb_signal_from_host (exit_signal));
+	DWORD exit_code = current_event->u.ExitProcess.dwExitCode;
+	*ourstatus = native_exit_code_to_target_status (exit_code);
       }
       continue_last_debug_event (DBG_CONTINUE, debug_threads);
       break;