This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [win32] Fix suspend count handling



> -----Original Message-----
> From: alves.ped@gmail.com [mailto:alves.ped@gmail.com] On Behalf Of
> Pedro Alves
> Sent: Wednesday, November 21, 2007 2:43 PM
> To: gdb-patches@sourceware.org
> Cc: Pierre Muller
> Subject: Re: [win32] Fix suspend count handling
> 
> On Nov 21, 2007 11:25 AM, Pierre Muller wrote:
> >   Hi Pedro,
> >
> >   I agree with you that the ResumeThread should be called
> > only once, even if the suspend_count is > 1, but when I tested your
> test
> > application,
> > I obtained 4 as a result with the current gdb head.
> >   This is because in fact the thread suspend_count are
> > only restored after ContinueDebugEvent is called.
> 
> I suspect you didn't reproduce the exact steps I described, probably
> because I didn't describe it sufficiently.  Here's a test run:
  You are right indeed (see below) 
> $ gdb/gdb.exe main.exe
> GNU gdb 6.7.50.20071121
> Copyright (C) 2007 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show
> copying"
> and "show warranty" for details.
> This GDB was configured as "i686-pc-cygwin"...
> (gdb) list 37
> 32              printf ("SuspendThreadFailed\n");
> 33              return 1;
> 34            }
> 35
> 36
> 37        suspend_count = ResumeThread (h); /* set breakpoint here */
> 38
> 39        printf ("%lu\n", suspend_count); /* should be 3 */
> 40
> 41        while ((suspend_count = ResumeThread (h)) != 0
> (gdb) b 37
> Breakpoint 1 at 0x40119a: file main.c, line 37.
> (gdb) r
> Starting program: /d/gdb/build/main.exe
> 
> Breakpoint 1, main (argc=1, argv=0x6b3270) at main.c:37
> 37        suspend_count = ResumeThread (h); /* set breakpoint here */
> (gdb) info threads
>   3 thread 11344.0x2e54  0x7c90eb94 in ntdll!LdrAccessResource () from
> /cygdrive/c/WINNT/system32/ntdll.dll
>   2 thread 11344.0x1e4c  0x7c90eb94 in ntdll!LdrAccessResource () from
> /cygdrive/c/WINNT/system32/ntdll.dll
> * 1 thread 11344.0x2d00  main (argc=1, argv=0x6b3270) at main.c:37

  This comes from the fact that cygwin startup code generated a thread
dedicated to signal handling,
but I figures that out after a while, while testing your patch...
> (gdb) thread 3
> [Switching to thread 3 (thread 11344.0x2e54)]#0  0x7c90eb94 in
> ntdll!LdrAccessResource ()
>    from /cygdrive/c/WINNT/system32/ntdll.dll
> (gdb) thread 1
> [Switching to thread 1 (thread 11344.0x2d00)]#0  main (argc=1,
> argv=0x6b3270) at main.c:37
> 37        suspend_count = ResumeThread (h); /* set breakpoint here */
> (gdb) n
> 39        printf ("%lu\n", suspend_count); /* should be 3 */
> (gdb) p suspend_count
> $1 = 0
> (gdb)

  Your patch does indeed correct this to 3, which is what is wanted.
But if you use 'continue' instead of 'next' when you are at line
37, you will get '4' printed out instead of three, because
the main thread will have call SuspendThread before gdb does.
This is at least what happens on my computer.

> 
> You must switch to the suspended thread for gdb to call SuspendThread
> on it, otherwise, gdb will not try to ResumeThread it, and the bug
> isn't
> triggered.  I mentioned "thread 2" in the other mail, but as seen on
> the "info threads" result, the gdb tid of the suspended thread will
> vary.
> A proper testcase would switch to all threads one by one and then
> back to thread 1 to ensure the bug is triggered.
> 
> >   Thus the debugged event can possibly called
> > ResumeThread before gdb does so, thus obtaining a
> > Thread count of 4 instead of 0...
> >   The first answer that comes to mind would be to
> > called ResumeThread before calling ContinueDebugEvent,
> > but that would mean that other threads would be restarted before
> > the thread that caused the debug event, which is
> > probably also not good.
> >
> 
> Not true.  See here:
>   [The Win32 Debugging Application Programming Interface]
>   http://msdn2.microsoft.com/en-us/library/ms809754.aspx
> 
>   "When a debug event occurs, execution of that process is suspended
> until
>   the debugger calls the ContinueDebugEvent function. Consequently,
> all threads in
>   the process are suspended while the debugger is processing the debug
> event."
> 
> In fact my patch at:
>  [win32] Fix watchpoint support
>  http://sourceware.org/ml/gdb-patches/2007-11/msg00390.html
> 
> ... ensures that ContinueDebugEvent is always called last.

  OK, in that case it is indeed correct to do all the ResumeThread calls
before ContinueDebugEvent is called, and thus the suspend_count can be
replaced
by suspended Boolean value.
  But only the combination of the two patches will ensure to 
always obtain '3' using 'continue' or 'next'.


Pierre




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]