[PATCH v2 08/11] Windows gdb+gdbserver: Decode Cygwin ExitProcess codes

[Pedro Alves <pedro@palves.net>]

On native Cygwin, GDB misreports the inferior's exit reason in several
common cases, resulting in several gdb.base/exitsignal.exp failures:

 $ grep FAIL gdb.sum
 FAIL: gdb.base/exitsignal.exp: how=run: signal: program terminated with SIGSEGV (the program exited)
 FAIL: gdb.base/exitsignal.exp: how=run: signal: $_exitsignal is 11 (SIGSEGV) after SIGSEGV.
 FAIL: gdb.base/exitsignal.exp: how=run: signal: $_exitcode is still void after SIGSEGV
 FAIL: gdb.base/exitsignal.exp: how=run: signal: $_exitsignal is 11 (SIGSEGV) after restarting the inferior
 FAIL: gdb.base/exitsignal.exp: how=run: signal: $_exitcode is still void after restarting the inferior
 FAIL: gdb.base/exitsignal.exp: how=run: normal: continue to exit
 FAIL: gdb.base/exitsignal.exp: how=run: normal: $_exitcode is one after normal inferior is executed
 FAIL: gdb.base/exitsignal.exp: how=run: normal: $_exitsignal is still void after normal inferior is executed
 FAIL: gdb.base/exitsignal.exp: how=attach: normal: continue to exit (the program exited)
 FAIL: gdb.base/exitsignal.exp: how=attach: normal: $_exitcode is one after normal inferior is executed

For example, from gdb.log, the normal exit case:

 ...
 [Thread 14300.0x4214 (id 1) exited with code 1]
 [Thread 14300.0x1b1c (id 4) exited with code 1]
 [Thread 14300.0x1e2c (id 2) exited with code 1]

 Program terminated with signal SIGHUP, Hangup.
 The program no longer exists.
 (gdb) FAIL: gdb.base/exitsignal.exp: how=run: normal: continue to exit

The program in fact exited normally with code 1.  SIGHUP happens to be
signal 1, and GDB picked the wrong interpretation.

Similarly, for the signal termination case:

 ...
 continue
 Continuing.
 [Thread 4600.0x3104 (id 4) exited with code 2816]
 [Thread 4600.0x2bcc (id 3) exited with code 2816]
 [Thread 4600.0x2f44 (id 1) exited with code 2816]
 [Inferior 1 (process 4600) exited with code 05400]
 (gdb) FAIL: gdb.base/exitsignal.exp: how=run: signal: program terminated with SIGSEGV (the program exited)

Here the inferior died with SIGSEGV, but GDB reported exit decimal
2816 / octal 05400 / hex 0x0B00, which is SIGSEGV swapped into the
high byte of a waitpid exit status.

The problem is that Cygwin waitpid exit status and Windows exit codes
do not have the same encoding, and GDB & GDBserver do not know about
this.

This commit fixes it.  It adds a Cygwin-specific branch to the code
that determines the terminating signal and status of a program.  The
branch for native Windows/MinGW GDB is left intact, no behavior change
there.

The way to decode the exit codes is a little bit tricky, see detailed
comments added by the patch.  To exercise the "raw NTSTATUS error
code" path in windows_process_info::exit_process_to_target_status,
gdb.base/exitsignal.exp is extended to debug a native Windows program
that crashes with a segfault (STATUS_ACCESS_VIOLATION).

With this, gdb.base/exitsignal.exp passes cleanly on Cygwin.

Change-Id: Icaebcc234b71927915c996fd120884604441415b
commit-id: bd0fbb9c
---
 gdb/nat/windows-nat.c                 | 153 +++++++++++++++++++++++++-
 gdb/nat/windows-nat.h                 |  29 +++++
 gdb/testsuite/gdb.base/exitsignal.exp |  32 +++++-
 gdb/windows-nat.c                     |   5 +
 gdbserver/win32-low.cc                |   5 +
 5 files changed, 217 insertions(+), 7 deletions(-)

diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index 92f9394ca6d..27bc65225cc 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -654,6 +654,7 @@ windows_process_info::add_dll (LPVOID load_addr)
 	 at which the DLL was loaded is equal to LOAD_ADDR.  */
       if (!(load_addr != nullptr && mi.lpBaseOfDll != load_addr))
 	{
+	  maybe_note_cygwin1_dll (name);
 	  handle_load_dll (name, mi.lpBaseOfDll);
 	  if (load_addr != nullptr)
 	    return;
@@ -681,7 +682,10 @@ windows_process_info::dll_loaded_event (const DEBUG_EVENT &current_event)
      by enumerating all the DLLs loaded into the inferior, looking for
      one that is loaded at base address = lpBaseOfDll. */
   if (dll_name != nullptr)
-    handle_load_dll (dll_name, event->lpBaseOfDll);
+    {
+      maybe_note_cygwin1_dll (dll_name);
+      handle_load_dll (dll_name, event->lpBaseOfDll);
+    }
   else if (event->lpBaseOfDll != nullptr)
     add_dll (event->lpBaseOfDll);
 }
@@ -694,6 +698,48 @@ windows_process_info::add_all_dlls ()
   add_dll (nullptr);
 }
 
+#ifdef __CYGWIN__
+
+/* See nat/windows-nat.h.  */
+
+void
+windows_process_info::maybe_note_cygwin1_dll (const char *dll_path)
+{
+  const char *base = dll_path + strlen (dll_path);
+  while (base > dll_path && base[-1] != '/' && base[-1] != '\\')
+    base--;
+  if (strcasecmp (base, "cygwin1.dll") == 0)
+    cygwin1_dll_loaded = true;
+}
+
+/* See nat/windows-nat.h.  */
+
+bool
+inferior_started_by_cygwin (DWORD winpid, bool attaching)
+{
+  /* In the run (non-attach) case this is called early when the
+     inferior has only just reached its first instruction and
+     cygwin1.dll hasn't initialized itself yet -- GDB launched the
+     inferior with raw CreateProcess, not through Cygwin's fork/spawn
+     path, so PID_CYGPARENT is necessarily false, so we can shortcut
+     without calling Cygwin.  */
+  if (!attaching)
+    return false;
+
+  /* Note CW_WINPID_TO_CYGWIN_PID never fails.  It returns a synthetic
+     pid for non-Cygwin or unknown winpids, in which case CW_GETPINFO
+     returns either a pinfo with PID_CYGPARENT unset, or NULL.  */
+  auto cygpid = (pid_t) cygwin_internal (CW_WINPID_TO_CYGWIN_PID, winpid);
+
+  auto *pinfo = (external_pinfo *) cygwin_internal (CW_GETPINFO, cygpid);
+  if (pinfo == nullptr)
+    return false;
+
+  return (pinfo->process_state & PID_CYGPARENT) != 0;
+}
+
+#endif /* __CYGWIN__.  */
+
 /* See nat/windows-nat.h.  */
 
 target_waitstatus
@@ -703,6 +749,110 @@ windows_process_info::exit_process_to_target_status
   DWORD exit_code = info.dwExitCode;
   target_waitstatus tstatus;
 
+#ifdef __CYGWIN__
+  /* A Cygwin parent waiting on a Cygwin child via waitpid doesn't go
+     through GetExitCodeProcess / the Win32 exit code at all.  It
+     reads the child's wait status directly out of the child's Cygwin
+     pinfo (shared memory), set by pinfo::exit in
+     winsup/cygwin/pinfo.cc.  So sys/wait.h macros apply to that value
+     verbatim.
+
+     GDB, however, even though it is itself a Cygwin program, drives
+     its inferiors via the native Win32 debugger API: it spawns them
+     with CreateProcess (DEBUG_PROCESS), not via Cygwin's
+     fork/spawn/posix_spawn, and consumes
+     EXIT_PROCESS_DEBUG_EVENT.dwExitCode from WaitForDebugEvent rather
+     than calling waitpid.  That dwExitCode value comes from the
+     inferior's ExitProcess call.
+
+     What that value means depends on two orthogonal things:
+
+     1. Is the inferior a Cygwin process at all?  If not, dwExitCode
+	is a raw Win32 exit value.
+
+     2. For a Cygwin inferior, was it created through Cygwin's spawn
+	path?
+
+	- If not, cygwin1.dll's pinfo::exit byte-swaps the wait status
+	  on the way out, so that the meaningful exit value lands in
+	  the low byte where native Win32 consumers (cmd.exe's "echo
+	  %errorlevel%", and bare GetExitCodeProcess readers) expect
+	  it.  This is the case for Cygwin inferiors that we run, via
+	  CreateProcess.
+
+	- If yes, cygwin1.dll does not swap.  We see this case if we
+	  attach to an already-running process with a Cygwin parent.
+
+	See winsup/cygwin/pinfo.cc:
+
+	 int exitcode = self->exitcode & 0xffff;
+	 if (!self->cygstarted)
+	   exitcode = ((exitcode & 0xff) << 8) | ((exitcode >> 8) & 0xff);
+	 ...
+	 ExitProcess (exitcode);
+  */
+
+  /* The inferior may also exit with a raw NTSTATUS error code, e.g.,
+     STATUS_ACCESS_VIOLATION (0xc0000005), without going through the
+     pinfo::exit at all -- for example, if the unhandled-exception
+     filter didn't run, or for processes that don't link cygwin1.dll.
+     Detect those and map them the same way Cygwin's set_exit_code
+     does in winsup/cygwin/pinfo.cc.  */
+  if (exit_code >= 0xc0000000)
+    {
+      gdb_signal sig;
+      switch (exit_code)
+	{
+	case EXCEPTION_ACCESS_VIOLATION:
+	  sig = GDB_SIGNAL_SEGV;
+	  break;
+	case EXCEPTION_ILLEGAL_INSTRUCTION:
+	  sig = GDB_SIGNAL_ILL;
+	  break;
+	case STATUS_NO_MEMORY:
+	  sig = GDB_SIGNAL_BUS;
+	  break;
+	case STATUS_CONTROL_C_EXIT:
+	  sig = GDB_SIGNAL_INT;
+	  break;
+	default:
+	  /* Cygwin maps any other NTSTATUS to exit 127.  */
+	  tstatus.set_exited (127);
+	  return tstatus;
+	}
+      tstatus.set_signalled (sig);
+      return tstatus;
+    }
+
+  if (!this->cygwin1_dll_loaded)
+    {
+      /* Non-Cygwin inferior: dwExitCode is a raw Win32 exit value.
+	 Limit to 8 bits, like Cygwin does, matching what happens with
+	 Cygwin inferiors.  */
+      tstatus.set_exited (exit_code & 0xff);
+      return tstatus;
+    }
+
+  /* Note: when GDB attaches to a Cygwin inferior and the inferior is
+     then killed externally (e.g., taskkill /F with exit code 1), GDB
+     and Cygwin disagree.  Cygwin's parent waitpid reports WIFEXITED,
+     code=1; GDB reports SIGHUP (signal 1, no swap below because
+     started_by_cygwin).  Cygwin's parent distinguishes "pinfo::exit
+     ran" from "didn't run" via the child's wait pipe and only applies
+     the swap-undo for the former.  GDB has only dwExitCode and can't
+     tell.  This can't be solved without Cygwin's help.  OTOH, such an
+     external termination steps out of Cygwin and arguably falls into
+     undefined-behavior territory, so it is less important than the
+     other cases.  */
+
+  int wstatus = exit_code & 0xffff;
+  if (!this->started_by_cygwin)
+    wstatus = ((wstatus & 0xff) << 8) | ((wstatus >> 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.  */
@@ -712,6 +862,7 @@ windows_process_info::exit_process_to_target_status
     tstatus.set_exited (exit_code);
   else
     tstatus.set_signalled (gdb_signal_from_host (exit_signal));
+#endif
 
   return tstatus;
 }
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index d2e6adb4f40..2e4db9832ae 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -224,6 +224,23 @@ struct windows_process_info
   DWORD process_id = 0;
   DWORD main_thread_id = 0;
 
+#ifdef __CYGWIN__
+  /* True if the inferior was created through Cygwin's spawn path
+     (i.e., its Cygwin pinfo has PID_CYGPARENT set).  We need this at
+     exit time, but we cache it early when we start debugging the
+     inferior, because by exit time the inferior's Cygwin pinfo may
+     have been torn down (CW_GETPINFO returns NULL).  */
+  bool started_by_cygwin = false;
+
+  /* True if cygwin1.dll is loaded into the inferior.  */
+  bool cygwin1_dll_loaded = false;
+
+  /* If DLL_PATH is cygwin1.dll, set cygwin1_dll_loaded to true.  */
+  void maybe_note_cygwin1_dll (const char *dll_path);
+#else
+  void maybe_note_cygwin1_dll (const char *) {}
+#endif
+
 #ifdef __x86_64__
   /* The target is a WOW64 process */
   bool wow64_process = false;
@@ -353,6 +370,18 @@ struct windows_process_info
   int get_exec_module_filename (char *exe_name_ret, size_t exe_name_max_len);
 };
 
+#ifdef __CYGWIN__
+/* Return true if the process with native Windows pid WINPID was
+   started by a Cygwin parent -- that is, its Cygwin pinfo exists and
+   has PID_CYGPARENT set.  Returns false if the process is not a
+   Cygwin process at all, or if its parent is not a Cygwin process.
+
+   ATTACHING indicates whether GDB is attaching to an already-running
+   inferior (true) or has just launched it via CreateProcess
+   (false).  */
+extern bool inferior_started_by_cygwin (DWORD winpid, bool attaching);
+#endif
+
 /* Return a string version of EVENT_CODE.  */
 
 extern std::string event_code_to_string (DWORD event_code);
diff --git a/gdb/testsuite/gdb.base/exitsignal.exp b/gdb/testsuite/gdb.base/exitsignal.exp
index 7684646b546..348c7a72eff 100644
--- a/gdb/testsuite/gdb.base/exitsignal.exp
+++ b/gdb/testsuite/gdb.base/exitsignal.exp
@@ -37,6 +37,10 @@ set exec2 "normal"
 set srcfile2 ${exec2}.c
 set binfile2 [standard_output_file ${exec2}]
 
+set exec3 "segv-win32"
+set srcfile3 ${exec1}.c
+set binfile3 [standard_output_file ${exec3}]
+
 if { [build_executable "failed to build $exec1" ${exec1} "${srcfile1}" \
 	  {debug}] == -1 } {
     return -1
@@ -47,6 +51,16 @@ if { [build_executable "failed to build $exec2" ${exec2} "${srcfile2}" \
     return -1
 }
 
+# On Cygwin, also build a pure-Win32 segv binary, used to test that
+# GDB extracts the terminating SIGSEGV out of the 0xc0000005
+# (STATUS_ACCESS_VIOLATION) Windows exit code.
+if {[istarget "*-*-cygwin*"]} {
+    if { [build_executable "failed to build $exec3" ${exec3} "${srcfile3}" \
+	      {debug win32}] == -1} {
+	return -1
+    }
+}
+
 # Get the inferior under GDB's control in mode HOW ("run" or
 # "attach"), using BINFILE.  In "attach" mode, spawn the binary and
 # attach to it; in "run" mode, run to main.  In both modes, clear the
@@ -81,14 +95,14 @@ proc teardown {how} {
     }
 }
 
-proc test_signal {how} {
-    clean_restart $::exec1
+proc test_signal {how exec binfile} {
+    clean_restart $exec
 
     # Get the inferior under GDB's control.  But, before, change cwd
     # so the core file ends up in the output directory.
     set_inferior_cwd_to_output_dir
 
-    setup $how $::binfile1
+    setup $how $binfile
 
     # Get the inferior's PID for later.
     set pid [get_inferior_pid]
@@ -106,7 +120,8 @@ proc test_signal {how} {
     gdb_test "continue" "(Thread .*|Program) received signal SIGSEGV.*" \
 	"trigger SIGSEGV"
 
-    if {[istarget "*-*-mingw*"]} {
+    if {[istarget "*-*-mingw*"]
+	|| ([istarget "*-*-cygwin*"] && $binfile == $::binfile3)} {
 	# We're debugging a pure Win32 program with no SEH handler.  The
 	# previous continue caught the first-chance exception.  Now we
 	# catch the second-chance one.
@@ -140,7 +155,7 @@ proc test_signal {how} {
     } else {
 	with_test_prefix "reattach" {
 	    kill_wait_spawned_process $::test_spawn_id
-	    setup $how $::binfile1
+	    setup $how $binfile
 	}
     }
 
@@ -190,9 +205,14 @@ foreach_with_prefix how {"run" "attach"} {
     }
 
     with_test_prefix "signal" {
-	test_signal $how
+	test_signal $how $exec1 $binfile1
     }
     with_test_prefix "normal" {
 	test_normal $how
     }
+    if {[istarget "*-*-cygwin*"]} {
+	with_test_prefix "signal, win32" {
+	    test_signal $how $exec3 $binfile3
+	}
+    }
 }
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 26333238cfa..16269671d95 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1994,6 +1994,11 @@ windows_nat_target::do_initial_windows_stuff (DWORD pid, bool attaching)
      phase, and then process them all in one batch now.  */
   windows_process->add_all_dlls ();
 
+#ifdef __CYGWIN__
+  windows_process->started_by_cygwin
+    = inferior_started_by_cygwin (pid, attaching);
+#endif
+
   windows_process->windows_initialization_done = 1;
   return;
 }
diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
index 550994b0bf2..2150045e188 100644
--- a/gdbserver/win32-low.cc
+++ b/gdbserver/win32-low.cc
@@ -368,6 +368,11 @@ do_initial_child_stuff (HANDLE proch, DWORD pid, int attached)
      phase, and then process them all in one batch now.  */
   windows_process.add_all_dlls ();
 
+#ifdef __CYGWIN__
+  windows_process.started_by_cygwin
+    = inferior_started_by_cygwin (pid, attached);
+#endif
+
   windows_process.child_initialization_done = 1;
 }
 
-- 
2.53.0