Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix.

[Pedro Alves <pedro_alves@portugalmail.pt>]

[-- Attachment #1: Type: text/plain, Size: 4508 bytes --]

Lerele wrote:
> Pedro Alves wrote:
> 
>> Space around '?' and ':'.  Personally, I liked the res != FALSE ? 0 : 
>> -1 version better.
>>
>>
>> Honestly, I don't really care too much.
>> You guys decide.
>>
>>
>> I don't care much either, but I think the spaces around '?' and ':'
>> are mandatory.
>>
>> +  return res ? 0 : -1;
> 
> 
> I was wrong, res is not a Win32 BOOL, it's plainly a 0 or a 1.

Then make it a BOOL, it makes the code clearer anyway.  See attachment.


> Already fixed spacing, while you were uploading your patches.
> I'll send a new version for interrupt patch when I get some time to do 
> it, I'll need get latest cvs and merge my changes, then resend patch.
> 

> I guess that would have been sufficient. But since you already changed 
> "target->send_signal" to "target->send_interrupt" I guess that is no 
> longer necessary.
> 

Yep, just change it to win32_send_interrupt (void), and drop the
if (sig == SIGINT) test.

>> I plan on eventually adding a third option to GenerateConsoleCtrlEvent
>> and DebugBreakProcess, that injects a thread into the debuggee, since
>> WinCE doesn't have any of the above.  That should make every
>> stoppable process stoppable, in all Windows versions.
>>
> 
> Do you find appropriate to use this method?

It is exactly what DebugBreakProcess does internally, so if you
have a problem with the approach, you shouldn't be using it :)
The thread is short-lived.  It is something like:

//inferior's address space
void helper_thread (void*)
{
   DebugBreak ();  // stops here.

   // and dies on resume.
}


// debugger's address space
DebugBreakProcess (pid)
{
   if (debugger_present) // we are sure of this
	                // but this call can come from
	                // a 3rd process.
     {
       create_remote_thread (helper_thread, pid);
     }
}


I stumbled on the fact that on WinCE there is no
CreateRemoteThread.  There is an undocumented but widely used
PerformCallback4 that does a pretty good hack and can be used
to simulate CreateRemoteThread (haven't tested yet), but it
gone in WinCE >= 5, and I don't know the a replacement, that
doesn't need messing with the registry to inject a
dll into the inferior's address space, and start a thread
on DllMain.  Still searching.  Maybe it is possible
to stop one thread, and manually manipulate the stack to
insert a call into CreateThread with DebugBreak as
entry point.  Although the prototype for DebugBreak is
different from what CreateThread expects, there should be
no problem, since WinCE is __cdecl.  It may be problematic
to do this is a thread blocked inside a Win32 or kernel
call, or if the inferior thread chosen to do it is in a stack
corrupted state.  The advantage of these approaches is that there
is a real breakpoint exception being thrown.  Oh, you can't just
insert a breakpoint at the current PC, since it may be a ROM
address (real Flash/ROM, not locked pages).

> Isn't there any other way to do it on WinCE?
> 

Probably.  I would like to know how other debuggers do it.

> 1) May be solved by doing:
> 
> 1. Create in gdbserver process as much threads as number of cpus -1 the 
> computer has. These threads should consume all scheduled cpu for them.
> 2. SetPriorityClass on gdbserver process with real-time priority.
> 3. GetPriorityClass on child and store it to restore later on.
> 4. SetPriorityClass on child with below normal or idle.
> 5. Suspend all child threads.
> Child should be stopped here.
> 
> Only problem I see with this can be if child changes its own priority 
> class between steps 3 and 4 above, however this is a very remote 
> possibility, because if this happens it is because child is already 
> running in real-time priority, and in this case gdbserver possibly may 
> not even work at all (unless you run gdbserver itself with this priority).
> 
> What do you think about this?

I was leaving something like this as a last resort, but should be possible,
I guess.  Would need extra logic to handle the fact that there isn't
any real breakpoint exception live (in resume, for instance).

> Does WinCE API have these required functions?
> It may seem a quite odd way of doing it, but if it works, that's what 
> counts I think.
> 

Don't know about WinCE > 5.  The whole security model + address space model
has changed.  I'm still looking at a portable way across all WinCEs from
Mobile 2003 to 6, and if I can simulate a DebugBreakProcess, it would
all be nicely encapsulated.

Thanks for the suggestions!

Cheers,
Pedro Alves

[-- Attachment #2: win32_attach --]
[-- Type: text/plain, Size: 1338 bytes --]

Index: src/gdb/gdbserver/win32-i386-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-i386-low.c	2007-03-03 19:52:56.000000000 +0000
+++ src/gdb/gdbserver/win32-i386-low.c	2007-03-05 00:01:52.000000000 +0000
@@ -673,7 +673,7 @@ win32_create_inferior (char *program, ch
 static int
 win32_attach (unsigned long pid)
 {
-  int res = 0;
+  BOOL res = FALSE;
   winapi_DebugActiveProcessStop *DebugActiveProcessStop = NULL;
 
   winapi_DebugSetProcessKillOnExit *DebugSetProcessKillOnExit = NULL;
@@ -683,7 +683,7 @@ win32_attach (unsigned long pid)
   DebugSetProcessKillOnExit = (winapi_DebugSetProcessKillOnExit *)
     GetProcAddress (kernel32, _T("DebugSetProcessKillOnExit"));
 
-  res = DebugActiveProcess (pid) ? 1 : 0;
+  res = DebugActiveProcess (pid);
 
   if (!res)
     error ("Attach to process failed.");
@@ -696,7 +696,7 @@ win32_attach (unsigned long pid)
 
   if (current_process_handle == NULL)
     {
-      res = 0;
+      res = FALSE;
       if (DebugActiveProcessStop != NULL)
 	DebugActiveProcessStop (current_process_id);
     }
@@ -707,7 +707,7 @@ win32_attach (unsigned long pid)
   if (kernel32 != NULL)
     FreeLibrary (kernel32);
 
-  return res? 0:-1;
+  return (res != FALSE) ? 0 : -1;
 }
 
 /* Handle OUTPUT_DEBUG_STRING_EVENT from child process.  */