[RFC] win32-nat.c: Improve handling of 'set shell on'

["Pierre Muller" <muller at ics dot u-strasbg dot fr>]

  I recently discovered that if you use
'set shell on' on cygwin native, there are 
some specific problems that appear.

  I ran the testsuite with a single line 
modified:
-static int useshell = 0;               /* use shell for subprocesses */
+static int useshell = 1;               /* use shell for subprocesses */
Which made the use of shell to start the subprocess
the default.
  The testuite results gave this:

--- gdb-cvs.sum 2008-01-08 11:09:16.684999100 +0100
+++ gdb-useshell.sum    2008-01-08 12:22:11.434999100 +0100
<<<< Middle cut >>>
                === gdb Summary ===

-# of expected passes           10522
-# of unexpected failures       514
+# of expected passes           10505
+# of unexpected failures       534
 # of expected failures         59
 # of known failures            31
 # of unresolved testcases      38

 Showing that you get 20 unexpected failures
more with 'use shell'
 I finally traced the problem down to the fact that
saw_create can sometimes be messed up by 
executables that launch other subprocesses.
 After adding two new variables holding the main process
id and the shell process id, I changed the code to check that the processed
of the 
event reported matches the main_process_id, otherwise
only checking for an EXIT_PROCESS_DEBUG_EVENT for shell_process_id,
even though I do not know if it would be acceptable
that the calling shell exits before the main process.

 After that change the testsuite results are much closer
to the results I get without using 'set shell on':
                === gdb Summary ===

-# of expected passes           10522
-# of unexpected failures       514
+# of expected passes           10517
+# of unexpected failures       519
 # of expected failures         59
 # of known failures            31
 # of unresolved testcases      38

Only 5 more failures instead of 20.

I send below the patch as I used it to check the testsuite results,
the second change, which sets the use of shell as a default,
would of course not be in my final patch...

Pierre Muller

PS: Is it possible to run the testsuite with a custom
script added ? This would facilitate testing of
other such options like 'set new-group'?
  The testsuite uses --nx option which disables
reading of .gdbinit file, but is it possible to 
add some other default option?
like "-x MySpecficOptionsFile".

 Can that be set? How?
Which tests would fail because of this?


ChangeLog entry:

2008-01-09  Pierre Muller  <muller@ics.u-strasbg.fr>

	* win32-nat.c (main_process_id, shell_process_id) New variables.
	// Not to be committed: Set default value of useshell to 1
	(fake_create_process): Set main_process_id.
	(get_win32_debug_event): only handle event if 
	current_event.dwProcessId is equal to main_process_id,
	and EXIT_PROCESS_DEBUG_EVENT event if
	current_event.dwProcessId is equal to shell_process_id.
	(do_initial_win32_stuff) Set shell_process_id or
	main_process_id depending on saw_create value.
	(win32_mourn_inferior) wait until EXIT_PROCESS_DEBUG_EVENT
	for shell_process_id is seen if shell_process_id is set.
	(win32_kill_inferior) wait for EXIT_PROCESS_DEBUG_EVENT
	for main_process_id and shell_process_id if shell_process_id is set.

Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.146
diff -u -p -r1.146 win32-nat.c
--- gdb/win32-nat.c     6 Jan 2008 06:59:14 -0000       1.146
+++ gdb/win32-nat.c     9 Jan 2008 08:38:57 -0000
@@ -137,6 +137,8 @@ static DEBUG_EVENT current_event;   /* The
 static HANDLE current_process_handle;  /* Currently executing process */
 static thread_info *current_thread;    /* Info on currently selected thread
*/
 static DWORD main_thread_id;           /* Thread ID of the main thread */
+static DWORD main_process_id;          /* Process ID of debugged process */
+static DWORD shell_process_id;         /* Process ID of the shell */

 /* Counts of things. */
 static int exception_count = 0;
@@ -154,7 +156,7 @@ static int debug_exec = 0;          /* show exec
 static int debug_events = 0;           /* show events from kernel */
 static int debug_memory = 0;           /* show target memory accesses */
 static int debug_exceptions = 0;       /* show target exceptions */
-static int useshell = 0;               /* use shell for subprocesses */
+static int useshell = 1;               /* use shell for subprocesses */

 /* This vector maps GDB's idea of a register's number into an address
    in the win32 exception context vector.
@@ -1176,6 +1178,7 @@ fake_create_process (void)
       /*  We can not debug anything in that case.  */
     }
   main_thread_id = current_event.dwThreadId;
+  main_process_id = current_event.dwProcessId;
   current_thread = win32_add_thread (main_thread_id,
                                     current_event.u.CreateThread.hThread);
   return main_thread_id;
@@ -1295,19 +1298,23 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "CREATE_THREAD_DEBUG_EVENT"));
-      if (saw_create != 1)
+      if (saw_create < 1)
        {
          if (!saw_create && attach_flag)
            {
              /* Kludge around a Windows bug where first event is a create
                 thread event.  Caused when attached process does not have
                 a main thread. */
+
              retval = ourstatus->value.related_pid = fake_create_process
();
-            if (retval)
-              saw_create++;
+             if (retval)
+               saw_create++;
            }
          break;
        }
+      if (current_event.dwProcessId != main_process_id)
+       break;
+
       /* Record the existence of this thread */
       th = win32_add_thread (current_event.dwThreadId,
                             current_event.u.CreateThread.hThread);
@@ -1323,6 +1330,8 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "EXIT_THREAD_DEBUG_EVENT"));
+      if (current_event.dwProcessId != main_process_id)
+       break;
       if (current_event.dwThreadId != main_thread_id)
        {
          win32_delete_thread (current_event.dwThreadId);
@@ -1339,6 +1348,7 @@ get_win32_debug_event (int pid, struct t
       if (++saw_create != 1)
        break;

+      main_process_id = current_event.dwProcessId;
       current_process_handle = current_event.u.CreateProcessInfo.hProcess;
       if (main_thread_id)
        win32_delete_thread (main_thread_id);
@@ -1354,8 +1364,16 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "EXIT_PROCESS_DEBUG_EVENT"));
-      if (saw_create != 1)
-       break;
+      if (current_event.dwProcessId != main_process_id)
+       {
+         if (saw_create > 1)
+           --saw_create;
+         if (current_event.dwProcessId == shell_process_id)
+           shell_process_id = 0;
+         break;
+       }
+      if (shell_process_id)
+       main_process_id = 0;
       ourstatus->kind = TARGET_WAITKIND_EXITED;
       ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
       retval = main_thread_id;
@@ -1367,7 +1385,7 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwThreadId,
                     "LOAD_DLL_DEBUG_EVENT"));
       CloseHandle (current_event.u.LoadDll.hFile);
-      if (saw_create != 1)
+      if (current_event.dwProcessId != main_process_id)
        break;
       catch_errors (handle_load_dll, NULL, (char *) "", RETURN_MASK_ALL);
       ourstatus->kind = TARGET_WAITKIND_LOADED;
@@ -1380,7 +1398,7 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "UNLOAD_DLL_DEBUG_EVENT"));
-      if (saw_create != 1)
+      if (current_event.dwProcessId != main_process_id)
        break;
       catch_errors (handle_unload_dll, NULL, (char *) "", RETURN_MASK_ALL);
       ourstatus->kind = TARGET_WAITKIND_LOADED;
@@ -1393,7 +1411,7 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "EXCEPTION_DEBUG_EVENT"));
-      if (saw_create != 1)
+      if (current_event.dwProcessId != main_process_id)
        break;
       switch (handle_exception (ourstatus))
        {
@@ -1415,13 +1433,13 @@ get_win32_debug_event (int pid, struct t
                     (unsigned) current_event.dwProcessId,
                     (unsigned) current_event.dwThreadId,
                     "OUTPUT_DEBUG_STRING_EVENT"));
-      if (saw_create != 1)
+      if (current_event.dwProcessId != main_process_id)
        break;
       retval = handle_output_debug_string (ourstatus);
       break;

     default:
-      if (saw_create != 1)
+      if (current_event.dwProcessId != main_process_id)
        break;
       printf_unfiltered ("gdb: kernel event for pid=%ld tid=%ld\n",
                         (DWORD) current_event.dwProcessId,
@@ -1431,7 +1449,7 @@ get_win32_debug_event (int pid, struct t
       break;
     }

-  if (!retval || saw_create != 1)
+  if (!retval || saw_create < 1)
     {
       if (continue_status == -1)
        win32_resume (ptid, 0, 1);
@@ -1486,6 +1504,14 @@ do_initial_win32_stuff (DWORD pid)
   extern int stop_after_trap;
   int i;

+  main_process_id = 0;
+  main_thread_id = 0;
+  shell_process_id = 0;
+
+  if (saw_create == -1)
+    shell_process_id = pid;
+  else
+    main_process_id = pid;
   last_sig = TARGET_SIGNAL_0;
   event_count = 0;
   exception_count = 0;
@@ -1893,7 +1919,39 @@ win32_create_inferior (char *exec_file,
 static void
 win32_mourn_inferior (void)
 {
+  /* If shell_process_id is set, we need to wait until
+     EXIT_PROCESS_DEBUG_EVENT for the shell process comes.  */
+  if (shell_process_id)
+    {
+      for (;;)
+       {
+         if (!win32_continue (DBG_CONTINUE, -1))
+           break;
+         if (!WaitForDebugEvent (&current_event, INFINITE))
+           break;
+         if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
+           {
+             DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%d
code=%s)\n",
+                           (unsigned) current_event.dwProcessId,
+                           (unsigned) current_event.dwThreadId,
+                           "EXIT_PROCESS_DEBUG_EVENT"));
+             if (current_event.dwProcessId == shell_process_id)
+               shell_process_id = 0;
+             if (current_event.dwProcessId == main_process_id)
+               main_process_id = 0;
+             if (main_process_id == 0 && shell_process_id == 0)
+               break;
+           }
+         else
+           DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%d code=%d)\n",
+                         (unsigned) current_event.dwProcessId,
+                         (unsigned) current_event.dwThreadId,
+                         (unsigned) current_event.dwDebugEventCode));
+        }
+    };
+
   (void) win32_continue (DBG_CONTINUE, -1);
+
   i386_cleanup_dregs();
   if (open_process_used)
     {
@@ -1956,7 +2014,25 @@ win32_kill_inferior (void)
       if (!WaitForDebugEvent (&current_event, INFINITE))
        break;
       if (current_event.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT)
-       break;
+       {
+         DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%d code=%s)\n",
+                       (unsigned) current_event.dwProcessId,
+                       (unsigned) current_event.dwThreadId,
+                       "EXIT_PROCESS_DEBUG_EVENT"));
+
+         if (current_event.dwProcessId == shell_process_id)
+           shell_process_id = 0;
+         if (current_event.dwProcessId == main_process_id)
+           main_process_id = 0;
+         if (main_process_id == 0 && shell_process_id == 0)
+           break;
+       }
+      else
+       DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%d code=%d)\n",
+                     (unsigned) current_event.dwProcessId,
+                     (unsigned) current_event.dwThreadId,
+                     (unsigned) current_event.dwDebugEventCode));
+
     }

   target_mourn_inferior ();    /* or just win32_mourn_inferior? */