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

Pierre Muller
2008-01-09T09:05:06Z

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? */