This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[gdbserver/win32] (3/11) Fix suspend count handling
- From: Pedro Alves <pedro_alves at portugalmail dot pt>
- To: gdb-patches at sourceware dot org, Lerele <lerele at champenstudios dot com>
- Date: Mon, 12 Nov 2007 02:06:59 +0000
- Subject: [gdbserver/win32] (3/11) Fix suspend count handling
Hi,
As Leo reported earlier, the suspend_count handling breaks
applications that use SuspendThread on their own threads.
SuspendThread returns the previous suspend count of the
thread, and gdbserver is calling ResumeThread that many times.
If the Thread was already suspended, say, 3 times, gdbserver will
do one SuspendThread call (in thread_rec), and 4 ResumeThread
calls when resuming.
This patch fixes it by turning the suspend_count counter into
a suspended flag.
Here is a test that shows the problem:
>cat ~/suspendfix/main.c
#include <windows.h>
HANDLE started;
HANDLE stop;
DWORD WINAPI
thread_start (void *arg)
{
SetEvent (started);
WaitForSingleObject (stop, INFINITE);
return 0;
}
int
main (int argc, char **argv)
{
int i;
DWORD suspend_count;
started = CreateEvent (NULL, TRUE, FALSE, NULL);
stop = CreateEvent (NULL, TRUE, FALSE, NULL);
HANDLE h = CreateThread (NULL, 0, thread_start, NULL,
0, NULL);
WaitForSingleObject (started, INFINITE);
for (i = 0; i < 3; i++)
if (SuspendThread (h) == (DWORD) -1)
{
printf ("SuspendThreadFailed\n");
return 1;
}
suspend_count = ResumeThread (h); /* set breakpoint here */
printf ("%lu\n", suspend_count); /* should be 3 */
while ((suspend_count = ResumeThread (h)) != 0
&& suspend_count != -1)
;
SetEvent (stop);
WaitForSingleObject (h, INFINITE);
CloseHandle (h);
CloseHandle (started);
CloseHandle (stop);
return 0;
}
Set a breakpoint where it read "set breakpoint here", switch to the
other thread with "thread 2", effectivelly forcing a thread_rec
call, which calls SuspendThread, and sets suspend_count. Since the
win32 support can't resume with a different thread, so go back to
thread 1, and step. The suspend_count will be 0, instead of 3.
I have yet to turn this into a proper gdb test. Is that wanted? Where
shall I put it? gdb.base? gdb.thread? other?
Note: The native (gdb/win32-nat.c) support also has the same bug.
Regtested on a local i686-pc-cygwin gdbserver, and against
a arm-wince remote.
Cheers,
Pedro Alves
2007-11-12 Leo Zayas <lerele@champenstudios@com>
Pedro Alves <pedro_alves@portugalmail.pt>
* win32-low.h (win32_thread_info): Add descriptions to the
structure members . Replace `suspend_count' counter by a
`suspended' flag.
* win32-low.c (thread_rec): Update condition of when to get the
context from the inferior. Rely on ContextFlags being set if it
has already been retrieved. Only suspend the inferior thread if
we haven't already. Warn if that fails.
(continue_one_thread): s/suspend_count/suspended/. Only call
ResumeThread once. Warn if that fails.
---
gdb/gdbserver/win32-low.c | 28 ++++++++++++++++++++--------
gdb/gdbserver/win32-low.h | 9 ++++++++-
2 files changed, 28 insertions(+), 9 deletions(-)
Index: src/gdb/gdbserver/win32-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-low.c 2007-11-04 23:50:36.000000000 +0000
+++ src/gdb/gdbserver/win32-low.c 2007-11-04 23:51:44.000000000 +0000
@@ -105,10 +105,19 @@ thread_rec (DWORD id, int get_context)
return NULL;
th = inferior_target_data (thread);
- if (!th->suspend_count && get_context)
+ if (get_context && th->context.ContextFlags == 0)
{
- if (id != current_event.dwThreadId)
- th->suspend_count = SuspendThread (th->h) + 1;
+ if (!th->suspended)
+ {
+ if (SuspendThread (th->h) == (DWORD) -1)
+ {
+ DWORD err = GetLastError ();
+ OUTMSG (("warning: SuspendThread failed in thread_rec, "
+ "(error %d): %s\n", (int) err, strwinerror (err)));
+ }
+ else
+ th->suspended = 1;
+ }
(*the_low_target.get_thread_context) (th, ¤t_event);
}
@@ -259,10 +268,9 @@ continue_one_thread (struct inferior_lis
struct thread_info *thread = (struct thread_info *) this_thread;
int thread_id = * (int *) id_ptr;
win32_thread_info *th = inferior_target_data (thread);
- int i;
if ((thread_id == -1 || thread_id == th->tid)
- && th->suspend_count)
+ && th->suspended)
{
if (th->context.ContextFlags)
{
@@ -270,9 +278,13 @@ continue_one_thread (struct inferior_lis
th->context.ContextFlags = 0;
}
- for (i = 0; i < th->suspend_count; i++)
- (void) ResumeThread (th->h);
- th->suspend_count = 0;
+ if (ResumeThread (th->h) == (DWORD) -1)
+ {
+ DWORD err = GetLastError ();
+ OUTMSG (("warning: ResumeThread failed in continue_one_thread, "
+ "(error %d): %s\n", (int) err, strwinerror (err)));
+ }
+ th->suspended = 0;
}
return 0;
Index: src/gdb/gdbserver/win32-low.h
===================================================================
--- src.orig/gdb/gdbserver/win32-low.h 2007-11-04 23:50:36.000000000 +0000
+++ src/gdb/gdbserver/win32-low.h 2007-11-04 23:51:16.000000000 +0000
@@ -22,9 +22,16 @@
each thread. */
typedef struct win32_thread_info
{
+ /* The Win32 thread identifier. */
DWORD tid;
+
+ /* The handle to the thread. */
HANDLE h;
- int suspend_count;
+
+ /* Non zero if SuspendThread was called on this thread. */
+ int suspended;
+
+ /* The context of the thread. */
CONTEXT context;
} win32_thread_info;