Patch : gdbserver get_image_name on CE

Patch : gdbserver get_image_name on CE

[Danny Backx]

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

Hi,

gdbserver doesn't work well yet on WinCE on x86. I'll send more fixes
when they are ready.

This one fixes obtaining the names of DLLs loaded by the inferior. The
patched code also still works on WinCE on ARM.

	Danny

2009-06-07  Danny Backx  <dannybackx@users.sourceforge.net>

	* win32-low.c (get_image_name) : Fix code to obtain DLL names,
	and document that the other code relies on sizeof(WCHAR) == 2.
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

[-- Attachment #2: gdbserver-get_image_name.patch --]
[-- Type: text/x-patch, Size: 2974 bytes --]

*** /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/win32-low.c	2008-02-14 23:41:39.000000000 +0100
--- win32-low.c	2009-06-07 11:12:30.000000000 +0200
***************
*** 894,907 ****
    loaded_dll (buf2, load_addr);
  }
  
  static char *
  get_image_name (HANDLE h, void *address, int unicode)
  {
!   static char buf[(2 * MAX_PATH) + 1];
    DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
    char *address_ptr;
    int len = 0;
!   char b[2];
    DWORD done;
  
    /* Attempt to read the name of the dll that was detected.
--- 894,912 ----
    loaded_dll (buf2, load_addr);
  }
  
+ /*
+  * Warning : some parts of this function rely on sizeof(WCHAR) == 2
+  */
  static char *
  get_image_name (HANDLE h, void *address, int unicode)
  {
!   static char buf[(2 * MAX_PATH) + 1]; /* here */
    DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
    char *address_ptr;
+ #ifndef _WIN32_WCE
    int len = 0;
!   char b[2]; /* here */
! #endif
    DWORD done;
  
    /* Attempt to read the name of the dll that was detected.
***************
*** 924,932 ****
      return NULL;
  #endif
  
    /* Find the length of the string */
    while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
! 	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
      continue;
  
    if (!unicode)
--- 929,956 ----
      return NULL;
  #endif
  
+ #ifdef _WIN32_WCE
+   /* Always unicode */
+   /* Assume you can read it all in one go, or otherwise the done variable will
+    * tell you how far you've read.
+    */
+   WCHAR *wbuf = alloca ((MAX_PATH + 1) * size);
+   ReadProcessMemory (h, address_ptr, wbuf, MAX_PATH * size, &done);
+   if (done < 0 || done > MAX_PATH * size)
+ 	  buf[0] = '\0';
+   else {
+     int n;
+     n = wcstombs (buf, wbuf, done);
+     if (n == (size_t)-1)
+       buf[0] = '\0';
+     /* No need to address the length limit case of the wcstombs call,
+      * buf has been allocated large enough. */
+   }
+   return buf;
+ #else
    /* Find the length of the string */
    while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
! 	 && (b[0] != 0 || b[size - 1] != 0) && done == size) /* here */
      continue;
  
    if (!unicode)
***************
*** 936,946 ****
        WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
        ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
  			 &done);
- 
        WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
      }
- 
    return buf;
  }
  
  typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
--- 960,969 ----
        WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
        ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
  			 &done);
        WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
      }
    return buf;
+ #endif
  }
  
  typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 07 June 2009 10:18:17, Danny Backx wrote:

> gdbserver doesn't work well yet on WinCE on x86. I'll send more fixes
> when they are ready.
> 
> This one fixes obtaining the names of DLLs loaded by the inferior. The
> patched code also still works on WinCE on ARM.

Could you please explain what is different here on x86 CE vs ARM CE
vs x86 desktop Windows?  Is x86 CE != ARM CE *and* != x86 desktop?

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

I can't give a full answer yet.

The code that I sent a patch for today didn't work. It reported a DLL
name "co" which should have been "coredll.dll". I suspect that the
result of ReadProcessMemory isn't always what it should be, causing the
loop to end too early. (This function also reports success via its last
parameter. Looks like that's more reliable.)

The next thing I'll do is figure out why the "bt" command isn't working.
Have no clue why. Will report when I do :-)

Feel free to ignore these patches until they're complete, if that is a
better idea. I sent this one separately because it looked contained.

	Danny

On Sun, 2009-06-07 at 18:04 +0100, Pedro Alves wrote:
> On Sunday 07 June 2009 10:18:17, Danny Backx wrote:
> 
> > gdbserver doesn't work well yet on WinCE on x86. I'll send more fixes
> > when they are ready.
> > 
> > This one fixes obtaining the names of DLLs loaded by the inferior. The
> > patched code also still works on WinCE on ARM.
> 
> Could you please explain what is different here on x86 CE vs ARM CE
> vs x86 desktop Windows?  Is x86 CE != ARM CE *and* != x86 desktop?
> 
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 07 June 2009 19:01:39, Danny Backx wrote:

> Feel free to ignore these patches until they're complete, if that is a
> better idea. 

It's fine to send unfinished patches for comments, if you'd like, but
please say so explicitly, so I don't waste time trying to understand
what they're for, for nothing.

> I sent this one separately because it looked contained. 

Thanks for doing that.  Splitting self contained patches is
always good.

In this case, x86 desktop and ARM CE are different in that:

- ARM CE stores dll names in unicode.
- ARM CE's dll event reports the address of the dll name.

- x86 desktop stores dll names in ascii.
- x86 desktop's dll event reports a pointer to the address of the dll name.

It may be that x86 CE is a mix of the two:

 - stores dll names in unicode:
 - x86 CE's dll event reports a pointer to the address
   of the dll name.

But this is just guessing.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sun, 2009-06-07 at 19:16 +0100, Pedro Alves wrote:
> It's fine to send unfinished patches for comments, if you'd like, but
> please say so explicitly, so I don't waste time trying to understand
> what they're for, for nothing.

This one isn't for nothing, it fixes a problem. It's just not all of the
work to get gdb/gdbserver to work for x86/ce .

> > I sent this one separately because it looked contained. 
> 
> Thanks for doing that.  Splitting self contained patches is
> always good.
> 
> In this case, x86 desktop and ARM CE are different in that:
> 
> - ARM CE stores dll names in unicode.
> - ARM CE's dll event reports the address of the dll name.
> 
> - x86 desktop stores dll names in ascii.
> - x86 desktop's dll event reports a pointer to the address of the dll name.
> 
> It may be that x86 CE is a mix of the two:

I don't think so.

CE -> unicode, both ARM and x86.
The DLL name was in the right place, it's just the code to read it
didn't work right.

As I said, I'm still trying to figure out what is happening, how x86/CE
works, etc. But CE on x86 looks similar to CE on ARM.

Except stuff like the packaging. None of the PocketPC type functionality
on the box that I'm using. But the box isn't a PDA so that's logical.

More info/patches when I have them ...

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 07 June 2009 20:12:47, Danny Backx wrote:
> On Sun, 2009-06-07 at 19:16 +0100, Pedro Alves wrote:
> > It's fine to send unfinished patches for comments, if you'd like, but
> > please say so explicitly, so I don't waste time trying to understand
> > what they're for, for nothing.
> 
> This one isn't for nothing, it fixes a problem. 

Sorry, you missed my point.  "for nothing" refered to my waste of time,
not to the usefulness of the patch.  Please do realize that if you don't
explain what the patch does, than whoever reviews it has to stare
at the code and try to make some sense of it.  If the patch
isn't obvious (this one isn't obvious to me), and doesn't
come with explanations, it is not going to be applied.  The onus
is on you to make it as easy as possible for a reviewer/maintainer to
accept a patch.  I really do not mean to sound harsh.  It is just
that it is easy to not realise that it does take time to review
a patch, and the backlog isn't getting shorter...

> It's just not all of the 
> work to get gdb/gdbserver to work for x86/ce .

I understand.

On Sunday 07 June 2009 20:12:47, Danny Backx wrote:
> CE -> unicode, both ARM and x86.
> The DLL name was in the right place, it's just the code to read it
> didn't work right.

It's the explanation of what's wrong with it that's missing.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sun, 2009-06-07 at 20:29 +0100, Pedro Alves wrote:
> On Sunday 07 June 2009 20:12:47, Danny Backx wrote:
> > On Sun, 2009-06-07 at 19:16 +0100, Pedro Alves wrote:
> > > It's fine to send unfinished patches for comments, if you'd like, but
> > > please say so explicitly, so I don't waste time trying to understand
> > > what they're for, for nothing.
> > 
> > This one isn't for nothing, it fixes a problem. 
> 
> Sorry, you missed my point.  "for nothing" refered to my waste of time,
> not to the usefulness of the patch.  Please do realize that if you don't
> explain what the patch does, than whoever reviews it has to stare
> at the code and try to make some sense of it.  If the patch
> isn't obvious (this one isn't obvious to me), and doesn't
> come with explanations, it is not going to be applied.  The onus
> is on you to make it as easy as possible for a reviewer/maintainer to
> accept a patch.  I really do not mean to sound harsh.  It is just
> that it is easy to not realise that it does take time to review
> a patch, and the backlog isn't getting shorter...
> 
> > It's just not all of the 
> > work to get gdb/gdbserver to work for x86/ce .
> 
> I understand.
> 
> On Sunday 07 June 2009 20:12:47, Danny Backx wrote:
> > CE -> unicode, both ARM and x86.
> > The DLL name was in the right place, it's just the code to read it
> > didn't work right.
> 
> It's the explanation of what's wrong with it that's missing.

Ok, thanks for pointing this out. I'll try to be clearer next time.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sun, 2009-06-07 at 19:16 +0100, Pedro Alves wrote:
> On Sunday 07 June 2009 19:01:39, Danny Backx wrote:
> 
> > Feel free to ignore these patches until they're complete, if that is a
> > better idea. 
> 
> It's fine to send unfinished patches for comments, if you'd like, but
> please say so explicitly, so I don't waste time trying to understand
> what they're for, for nothing.
> 
> > I sent this one separately because it looked contained. 
> 
> Thanks for doing that.  Splitting self contained patches is
> always good.

Excellent news : I just figured out why gdbserver was failing on CE/x86
where the same code worked for ARM.

The GetThreadContext call looked different between win32-i386-low.c and
its ARM equivalent :

  th->context.ContextFlags = 
    CONTEXT_FULL |
    CONTEXT_FLOATING_POINT |
    CONTEXT_EXTENDED_REGISTERS |
    CONTEXT_DEBUG_REGISTERS;
  GetThreadContext (th->h, &th->context);

On ARM, only CONTEXT_FULL and CONTEXT_FLOATING_POINT were used. The call
on x86 returned error 87 (ERROR_INVALID_PARAMETER) but MSDN doesn't
contain clues as to what that means.

I'll try to figure out whether this breaks anything, but at first sight
this small change (and a couple of others I have yet to submit) makes
gdbserver work.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

RE: Patch : gdbserver get_image_name on CE

[Johnny Willemsen]

Hi,

> Excellent news : I just figured out why gdbserver was failing on CE/x86
> where the same code worked for ARM.
> 
> The GetThreadContext call looked different between win32-i386-low.c and
> its ARM equivalent :
> 
>   th->context.ContextFlags =
>     CONTEXT_FULL |
>     CONTEXT_FLOATING_POINT |
>     CONTEXT_EXTENDED_REGISTERS |
>     CONTEXT_DEBUG_REGISTERS;
>   GetThreadContext (th->h, &th->context);
> 
> On ARM, only CONTEXT_FULL and CONTEXT_FLOATING_POINT were used. The
> call
> on x86 returned error 87 (ERROR_INVALID_PARAMETER) but MSDN doesn't
> contain clues as to what that means.

I have had this a few times also with other calls on CE, it seems you pass
an incorrect value that isn't supported.

Johnny

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

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

Ping ?

On Wed, 2009-07-08 at 13:33 +0200, Danny Backx wrote:
> On Wed, 2009-07-08 at 11:55 +0200, Danny Backx wrote:
> > It looks like ReadProcessMemory refuses to read from an address beyond
> > the pointer, but it does work from the pointer itself.
> > 
> > I've changed my code such that it loops, and tries to read a bigger
> > chunk of memory at each iteration.
> > 
> > I'm not bothering you with that code or its debug output right now.
> > 
> > In that way, the code appears to work, and it complies with the
> > documentation saying not to read more than required.
> > 
> > Shall I submit a new patch based on this ?
> > 
> > 	Danny
> 
> Ignore that question, here's the patch.
> 
> Please comment on its contents and on my adherence to the coding
> standards. Apologies for that, I seem to have a hard time learning that.
> 
> 	Danny
> 
> 2009-07-08  Danny Backx  <dannybackx@users.sourceforge.net>
> 
> 	* win32-low.c (get_image_name) : Work around ReadProcessMemory
> 	failure when reading at arbitrary addresses.
> 
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1649 bytes --]

? win32-low.c.ok
? x
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.38
diff -u -r1.38 win32-low.c
--- win32-low.c	4 Jul 2009 18:13:28 -0000	1.38
+++ win32-low.c	8 Jul 2009 11:29:25 -0000
@@ -922,7 +922,6 @@
   DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
   char *address_ptr;
   int len = 0;
-  char b[2];
   DWORD done;
 
   /* Attempt to read the name of the dll that was detected.
@@ -945,21 +944,23 @@
     return NULL;
 #endif
 
-  /* Find the length of the string */
-  while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
-	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
-    continue;
+  /* ReadProcessMemory sometimes fails when reading a (w)char at a time, but
+   * we can't just read MAX_PATH (w)chars either : msdn says not to cross the
+   * boundary into inaccessible areas.
+   * So we loop, reading more characters each time, until we find the NULL.
+   */
+  WCHAR *wbuf = alloca ((MAX_PATH + 1) * size);
+  while (1)
+    {
+      ReadProcessMemory (h, address_ptr, wbuf, ++len * size, &done);
+      if (wbuf[len - 1] == 0)
+        break;
+    }
 
   if (!unicode)
     ReadProcessMemory (h, address_ptr, buf, len, &done);
   else
-    {
-      WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
-      ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
-			 &done);
-
-      WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
-    }
+    WideCharToMultiByte (CP_ACP, 0, wbuf, len, buf, len, 0, 0);
 
   return buf;
 }

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

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

On Wed, 2009-07-08 at 11:55 +0200, Danny Backx wrote:
> It looks like ReadProcessMemory refuses to read from an address beyond
> the pointer, but it does work from the pointer itself.
> 
> I've changed my code such that it loops, and tries to read a bigger
> chunk of memory at each iteration.
> 
> I'm not bothering you with that code or its debug output right now.
> 
> In that way, the code appears to work, and it complies with the
> documentation saying not to read more than required.
> 
> Shall I submit a new patch based on this ?
> 
> 	Danny

Ignore that question, here's the patch.

Please comment on its contents and on my adherence to the coding
standards. Apologies for that, I seem to have a hard time learning that.

	Danny

2009-07-08  Danny Backx  <dannybackx@users.sourceforge.net>

	* win32-low.c (get_image_name) : Work around ReadProcessMemory
	failure when reading at arbitrary addresses.

-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1649 bytes --]

? win32-low.c.ok
? x
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.38
diff -u -r1.38 win32-low.c
--- win32-low.c	4 Jul 2009 18:13:28 -0000	1.38
+++ win32-low.c	8 Jul 2009 11:29:25 -0000
@@ -922,7 +922,6 @@
   DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
   char *address_ptr;
   int len = 0;
-  char b[2];
   DWORD done;
 
   /* Attempt to read the name of the dll that was detected.
@@ -945,21 +944,23 @@
     return NULL;
 #endif
 
-  /* Find the length of the string */
-  while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
-	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
-    continue;
+  /* ReadProcessMemory sometimes fails when reading a (w)char at a time, but
+   * we can't just read MAX_PATH (w)chars either : msdn says not to cross the
+   * boundary into inaccessible areas.
+   * So we loop, reading more characters each time, until we find the NULL.
+   */
+  WCHAR *wbuf = alloca ((MAX_PATH + 1) * size);
+  while (1)
+    {
+      ReadProcessMemory (h, address_ptr, wbuf, ++len * size, &done);
+      if (wbuf[len - 1] == 0)
+        break;
+    }
 
   if (!unicode)
     ReadProcessMemory (h, address_ptr, buf, len, &done);
   else
-    {
-      WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
-      ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
-			 &done);
-
-      WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
-    }
+    WideCharToMultiByte (CP_ACP, 0, wbuf, len, buf, len, 0, 0);
 
   return buf;
 }

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sat, 2009-07-04 at 19:15 +0100, Pedro Alves wrote:
> On Wednesday 01 July 2009 21:13:05, Pedro Alves wrote:
> > I'll see about cleaning up the bits of your patch that are OK and
> > apply them (you still aren't following the code conventions).  Then
> > we can focus on just this bit.
> 
> I've checked in the patch below, after giving it a testsuite
> run against a local Cygwin gdbserver without regressions.

I tried to debug gdbserver with gdbserver but the child didn't come to
life, don't know why, so I'm falling back to printf.

Changed the source (of get_image_name) in this way, output below.

#if 1
  /* Add debug code here */
  /* Find the length of the string */
  int r;
  while (1) {
          r = ReadProcessMemory (h, address_ptr + len++ * size, &b,
size, &done);
    if (r == 0) { 
            DWORD e = GetLastError(); 
            printf("ReadProcessMemory (in loop) r %d done %d -> %c %c,
error %d\n", 
                            r, done, b[0], b[1], e); 
    } else { 
            printf("ReadProcessMemory (in loop) r %d done %d -> %c %c
\n", 
                            r, done, b[0], b[1]);
    }
    if (len > 30)
            break;
         if ((b[0] != 0 || b[size - 1] != 0) && done == size)
    continue;
  }
    printf("ReadProcessMemory (after loop) r %d done %d -> %c\n",
                    r, done, b);

  if (!unicode)
    ReadProcessMemory (h, address_ptr, buf, len, &done);
  else
    {
      WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
      ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof
(WCHAR),
                         &done);

      WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0,
0);
    }
#else
  /* Original version, do not touch */
  /* Find the length of the string */


Output of this debugging session :

\network\x86> 
gdbserver :9999 /network/x86/hello2.exe 
Process /network/x86/hello2.exe created; pid = 92733450
Yow(libstdc++-6.dll)
ReadProcessMemory (in loop) r 1 done 2 -> l 
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (in loop) r 0 done 2 -> l , error 87
ReadProcessMemory (after loop) r 0 done 2 -> �
Listening on port 9999
Remote debugging from host 172.17.1.10
Detaching from process 92733450
\network\x86> 

Error 87 is ERROR_INVALID_PARAMETER. The MSDN page doesn't appear to
describe this.

It looks like ReadProcessMemory refuses to read from an address beyond
the pointer, but it does work from the pointer itself.

I've changed my code such that it loops, and tries to read a bigger
chunk of memory at each iteration.

I'm not bothering you with that code or its debug output right now.

In that way, the code appears to work, and it complies with the
documentation saying not to read more than required.

Shall I submit a new patch based on this ?

	Danny


-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Wednesday 01 July 2009 21:13:05, Pedro Alves wrote:
> I'll see about cleaning up the bits of your patch that are OK and
> apply them (you still aren't following the code conventions).  Then
> we can focus on just this bit.

I've checked in the patch below, after giving it a testsuite
run against a local Cygwin gdbserver without regressions.

-- 
Pedro Alves

2009-07-04  Danny Backx  <dannybackx@users.sourceforge.net>
	    Pedro Alves  <pedro@codesourcery.com>

	* win32-i386-low.c (i386_get_thread_context): Handle systems that
	don't support CONTEXT_EXTENDED_REGISTERS.
	(i386_win32_breakpoint, i386_win32_breakpoint_len): New.
	(the_low_target): Install them.
	* win32-low.c (get_child_debug_event): Handle WaitForDebugEvent
	failing with ERROR_PIPE_NOT_CONNECTED.

---
 gdb/gdbserver/win32-i386-low.c |   34 ++++++++++++++++++++++++++--------
 gdb/gdbserver/win32-low.c      |   17 ++++++++++++++++-
 2 files changed, 42 insertions(+), 9 deletions(-)

Index: src/gdb/gdbserver/win32-i386-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-i386-low.c	2009-07-04 17:39:07.046875000 +0100
+++ src/gdb/gdbserver/win32-i386-low.c	2009-07-04 19:11:08.375000000 +0100
@@ -129,13 +129,28 @@
 static void
 i386_get_thread_context (win32_thread_info *th, DEBUG_EVENT* current_event)
 {
-  th->context.ContextFlags = \
-    CONTEXT_FULL | \
-    CONTEXT_FLOATING_POINT | \
-    CONTEXT_EXTENDED_REGISTERS | \
-    CONTEXT_DEBUG_REGISTERS;
+  /* Requesting the CONTEXT_EXTENDED_REGISTERS register set fails if
+     the system doesn't support extended registers.  */
+  static DWORD extended_registers = CONTEXT_EXTENDED_REGISTERS;
+
+ again:
+  th->context.ContextFlags = (CONTEXT_FULL
+			      | CONTEXT_FLOATING_POINT
+			      | CONTEXT_DEBUG_REGISTERS
+			      | extended_registers);
 
-  GetThreadContext (th->h, &th->context);
+  if (!GetThreadContext (th->h, &th->context))
+    {
+      DWORD e = GetLastError ();
+
+      if (extended_registers && e == ERROR_INVALID_PARAMETER)
+	{
+	  extended_registers = 0;
+	  goto again;
+	}
+
+      error ("GetThreadContext failure %ld\n", (long) e);
+    }
 
   debug_registers_changed = 0;
 
@@ -283,6 +298,9 @@
   collect_register (r, context_offset);
 }
 
+static const unsigned char i386_win32_breakpoint = 0xcc;
+#define i386_win32_breakpoint_len 1
+
 struct win32_target_ops the_low_target = {
   init_registers_i386,
   sizeof (mappings) / sizeof (mappings[0]),
@@ -293,8 +311,8 @@
   i386_fetch_inferior_register,
   i386_store_inferior_register,
   i386_single_step,
-  NULL, /* breakpoint */
-  0, /* breakpoint_len */
+  &i386_win32_breakpoint,
+  i386_win32_breakpoint_len,
   i386_insert_point,
   i386_remove_point,
   i386_stopped_by_watchpoint,
Index: src/gdb/gdbserver/win32-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-low.c	2009-07-04 17:39:07.092875000 +0100
+++ src/gdb/gdbserver/win32-low.c	2009-07-04 19:00:07.750000000 +0100
@@ -1407,7 +1407,22 @@
 	 interruption, but high enough so gdbserver doesn't become a
 	 bottleneck.  */
       if (!WaitForDebugEvent (&current_event, 250))
-	return 0;
+        {
+	  DWORD e  = GetLastError();
+
+	  if (e == ERROR_PIPE_NOT_CONNECTED)
+	    {
+	      /* This will happen if the loader fails to succesfully
+		 load the application, e.g., if the main executable
+		 tries to pull in a non-existing export from a
+		 DLL.  */
+	      ourstatus->kind = TARGET_WAITKIND_EXITED;
+	      ourstatus->value.integer = 1;
+	      return 1;
+	    }
+
+	  return 0;
+        }
     }
 
  gotevent:

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Wednesday 01 July 2009 19:40:48, Danny Backx wrote:
> > And you never did explain what was broken with get_image_name
> > that you had to fix, as I asked $n emails ago, right?
> 
> Err. I am almost certain that the return value of GetProcessMemory isn't
> a reliable way to verify success. 

Thanks!  You know that I couldn't guess what you were thinking or were
almost certain, right?

> Its final parameter is. Or at least 
> that is my experience. Of course, MSDN docs don't mention this.

I'm not convinced of this.  I think you must stumbling on some other bug,
or a bug in the device.  MSDN docs mention this:

http://msdn.microsoft.com/en-us/library/ms680553(VS.85).aspx

"Reads data from an area of memory in a specified process. The entire area to be read
must be accessible or the operation fails."

That's why we read data bytewise to know the length or the string first.
So what you're doing may work for you, but it may fail for someone
else (other device, binary, etc.).

It makes *no sense* to be able to read the data in one go,
but not be able to read it bytewise.  This would mean that ReadProcessMemory
could fail in many other circumstances --- it is used when gdb wants to
read memory of off the inferior...

So, please, now that you have a gdbserver that works, debug gdbserver
with gdbserver, and step through that routine (the old implementation),
and see where/why it is failing.  Posting a log of that debug session
showing the failure would do wonders in convincing me where is the bug.

Hey, you may even have a compiler bug, right?

> I tested this on ARM and on i386. Real devices, not emulators. The ARM
> implementation worked as it was, and continued to work after my fix. The
> i386 version didn't work before, does work after the fix.
> 
> Is this sufficient explanation?

I'm not convinced yet.  :-)

I'll see about cleaning up the bits of your patch that are OK and
apply them (you still aren't following the code conventions).  Then
we can focus on just this bit.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

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

On Tue, 2009-06-30 at 22:58 +0100, Pedro Alves wrote:
> On Tuesday 30 June 2009 22:07:19, Danny Backx wrote:
> > 
> > > Is the rest of my patch acceptable or are there things I need to
> > > address ?
> > >
> 
> Did you get to confirm what really that ERROR_PIPE_NOT_CONNECTED
> is about?
> 
>  http://sourceware.org/ml/gdb-patches/2009-06/msg00373.html
> 
> Could you post an updated, cleaned up patch, without any extra
> unnecessary bits removed, along with change log entry, using
> 'cvs diff -up'?

Attached. Comments, as always, welcome.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

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

2009-07-01  Danny Backx  <dannybackx@users.sourceforge.net>

	* win32-i386-low.c (i386_get_thread_context): Avoid failure by
	calling GetThreadContext with flags that don't work on every
	processor. Fall back to querying less registers if that happens.
	* win32-i386-low.c (the_low_target, i386_wince_breakpoint) : Implement
	breakpoints.
	* win32-low.c (get_image_name): Don't rely on return value of
	ReadProcessMemory. Comment where some of the code still relies on
	sizeof(WCHAR) == 2.
	* win32-low.c (get_child_debug_event): Detect and work around a known
	case where CreateProcess reports success but the inferior dies
	immediately.

Index: win32-i386-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-i386-low.c,v
retrieving revision 1.14
diff -u -u -p -r1.14 win32-i386-low.c
--- win32-i386-low.c	3 Jan 2009 05:57:57 -0000	1.14
+++ win32-i386-low.c	1 Jul 2009 19:28:15 -0000
@@ -39,16 +39,36 @@ i386_initial_stuff (void)
   debug_registers_used = 0;
 }
 
+/*
+ * According to Mike Stall's .net debugging blog
+ * (http://blogs.msdn.com/jmstall/archive/2005/01/18/355697.aspx)
+ * the CONTEXT_EXTENDED_REGISTERS flag must be omitted if hardware doesn't
+ * support it. So I guess the only reasonable thing to do is just try.
+ */
 static void
 i386_get_thread_context (win32_thread_info *th, DEBUG_EVENT* current_event)
 {
-  th->context.ContextFlags = \
-    CONTEXT_FULL | \
-    CONTEXT_FLOATING_POINT | \
-    CONTEXT_EXTENDED_REGISTERS | \
+  /* try all flags */
+  th->context.ContextFlags =
+    CONTEXT_FULL |
+    CONTEXT_FLOATING_POINT |
+    CONTEXT_EXTENDED_REGISTERS |
     CONTEXT_DEBUG_REGISTERS;
 
-  GetThreadContext (th->h, &th->context);
+  if (GetThreadContext (th->h, &th->context) == 0) {
+    DWORD e = GetLastError();
+
+    if (e == ERROR_INVALID_PARAMETER) {
+      /* try limited set */
+      th->context.ContextFlags = CONTEXT_FULL |
+	CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS;
+      if (GetThreadContext (th->h, &th->context) == 0) {
+	DWORD e = GetLastError();
+	printf("GetThreadContext failure %d\n", e);
+	return;
+      }
+    }
+  }
 
   debug_registers_changed = 0;
 
@@ -193,6 +213,12 @@ i386_store_inferior_register (win32_thre
   collect_register (r, context_offset);
 }
 
+/*
+ * The INT 3 instruction is used for x86 platform breakpointing.
+ */
+static const unsigned char i386_wince_breakpoint = 0xCC;
+#define i386_wince_breakpoint_len 1
+
 struct win32_target_ops the_low_target = {
   init_registers_i386,
   sizeof (mappings) / sizeof (mappings[0]),
@@ -203,6 +229,6 @@ struct win32_target_ops the_low_target =
   i386_fetch_inferior_register,
   i386_store_inferior_register,
   i386_single_step,
-  NULL, /* breakpoint */
-  0, /* breakpoint_len */
+  &i386_wince_breakpoint, /* breakpoint */
+  i386_wince_breakpoint_len, /* breakpoint_len */
 };
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.35
diff -u -u -p -r1.35 win32-low.c
--- win32-low.c	1 Apr 2009 22:50:24 -0000	1.35
+++ win32-low.c	1 Jul 2009 19:28:15 -0000
@@ -873,14 +873,19 @@ win32_add_one_solib (const char *name, C
   loaded_dll (buf2, load_addr);
 }
 
+/*
+ * Warning : some parts of this function rely on sizeof(WCHAR) == 2
+ */
 static char *
 get_image_name (HANDLE h, void *address, int unicode)
 {
-  static char buf[(2 * MAX_PATH) + 1];
+  static char buf[(2 * MAX_PATH) + 1]; /* here */
   DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
   char *address_ptr;
+#ifndef _WIN32_WCE
   int len = 0;
-  char b[2];
+  char b[2]; /* here */
+#endif
   DWORD done;
 
   /* Attempt to read the name of the dll that was detected.
@@ -903,9 +908,28 @@ get_image_name (HANDLE h, void *address,
     return NULL;
 #endif
 
+#ifdef _WIN32_WCE
+  /* Always unicode */
+  /* Assume you can read it all in one go, or otherwise the done variable will
+   * tell you how far you've read.
+   */
+  WCHAR *wbuf = alloca ((MAX_PATH + 1) * size);
+  ReadProcessMemory (h, address_ptr, wbuf, MAX_PATH * size, &done);
+  if (done < 0 || done > MAX_PATH * size)
+	  buf[0] = '\0';
+  else {
+    int n;
+    n = wcstombs (buf, wbuf, done);
+    if (n == (size_t)-1)
+      buf[0] = '\0';
+    /* No need to address the length limit case of the wcstombs call,
+     * buf has been allocated large enough. */
+  }
+  return buf;
+#else
   /* Find the length of the string */
   while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
-	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
+	 && (b[0] != 0 || b[size - 1] != 0) && done == size) /* here */
     continue;
 
   if (!unicode)
@@ -920,6 +944,7 @@ get_image_name (HANDLE h, void *address,
     }
 
   return buf;
+#endif
 }
 
 typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
@@ -1365,7 +1390,20 @@ get_child_debug_event (struct target_wai
 	 interruption, but high enough so gdbserver doesn't become a
 	 bottleneck.  */
       if (!WaitForDebugEvent (&current_event, 250))
-	return 0;
+        {
+	  /*
+	   * Sometimes an application will just not start up.
+	   * Detect this here, return in such a way that the loop ends.
+	   */
+	  DWORD e = GetLastError();
+
+	  if (e == ERROR_PIPE_NOT_CONNECTED)
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_EXITED;
+	      return 1;	/* break the loop in our caller */
+	    }
+   	  return 0;
+        }
     }
 
  gotevent:

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Wed, 2009-07-01 at 19:53 +0100, Pedro Alves wrote:
> On Wednesday 01 July 2009 19:40:48, Danny Backx wrote:
> > I am certain that this happens when you create an executable which
> > attempts to get an API from a DLL by name, if the API happens not to be
> > in the DLL.
> > 
> > You'll probably need to read that five times to understand it :-)
> 
> Nope, I got it the first time.  :-)
> 
> > 
> > If the development environment is wrong in this way :
> > - a .def file was used to create a .dll.a file
> > - the .def file implements an api that's not actually in the DLL
> > 
> > This could be due to 
> > - an invalid .def file
> > - a .def file that's valid for one distribution of WinCE but not for
> >   another (read: differences between CE versions)
> > 
> > Anyway. What happens is the executable appears to start (CreateProcess
> > returns valid results), then the loader kicks in and fails. This is
> > where I have verified that you will get this result. 
> > 
> > Furthermore : none of the debug API's work either, gdbserver really gets
> > not a single sensible signal from the underlying process.
> > 
> 
> What I was interested in knowing, was if loading a similarly faulty
> dll (not the main executable) triggers the same issue.  Both in the
> case where the app is "linked" to it (gcc -o foo foo.c -lbaddll) ---
> I expect so, and in the case where LoadLibrary is used to pull in a
> bad dll.  If the latter case triggers the same "lost connection to
> inferior" error, then you have yourself a kernel bug.
> 
> You'll probably need to read that five times to understand it :-)

Hmm :-)

I verified the LoadLibrary case. I believe the LoadLibrary call returned
the right error message (ERROR_DLL_INIT_FAILED, 1114L).

I don't have the evidence of that test any more though.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Wednesday 01 July 2009 19:40:48, Danny Backx wrote:
> I am certain that this happens when you create an executable which
> attempts to get an API from a DLL by name, if the API happens not to be
> in the DLL.
> 
> You'll probably need to read that five times to understand it :-)

Nope, I got it the first time.  :-)

> 
> If the development environment is wrong in this way :
> - a .def file was used to create a .dll.a file
> - the .def file implements an api that's not actually in the DLL
> 
> This could be due to 
> - an invalid .def file
> - a .def file that's valid for one distribution of WinCE but not for
>   another (read: differences between CE versions)
> 
> Anyway. What happens is the executable appears to start (CreateProcess
> returns valid results), then the loader kicks in and fails. This is
> where I have verified that you will get this result. 
> 
> Furthermore : none of the debug API's work either, gdbserver really gets
> not a single sensible signal from the underlying process.
> 

What I was interested in knowing, was if loading a similarly faulty
dll (not the main executable) triggers the same issue.  Both in the
case where the app is "linked" to it (gcc -o foo foo.c -lbaddll) ---
I expect so, and in the case where LoadLibrary is used to pull in a
bad dll.  If the latter case triggers the same "lost connection to
inferior" error, then you have yourself a kernel bug.

You'll probably need to read that five times to understand it :-)

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Tue, 2009-06-30 at 22:58 +0100, Pedro Alves wrote:
> On Tuesday 30 June 2009 22:07:19, Danny Backx wrote:
> > 
> > > Is the rest of my patch acceptable or are there things I need to
> > > address ?
> > >
> 
> Did you get to confirm what really that ERROR_PIPE_NOT_CONNECTED
> is about?
> 
>  http://sourceware.org/ml/gdb-patches/2009-06/msg00373.html

I am certain that this happens when you create an executable which
attempts to get an API from a DLL by name, if the API happens not to be
in the DLL.

You'll probably need to read that five times to understand it :-)

If the development environment is wrong in this way :
- a .def file was used to create a .dll.a file
- the .def file implements an api that's not actually in the DLL

This could be due to 
- an invalid .def file
- a .def file that's valid for one distribution of WinCE but not for
  another (read: differences between CE versions)

Anyway. What happens is the executable appears to start (CreateProcess
returns valid results), then the loader kicks in and fails. This is
where I have verified that you will get this result. 

Furthermore : none of the debug API's work either, gdbserver really gets
not a single sensible signal from the underlying process.

> Could you post an updated, cleaned up patch, without any extra
> unnecessary bits removed, along with change log entry, using
> 'cvs diff -up'?

Will do.

> And you never did explain what was broken with get_image_name
> that you had to fix, as I asked $n emails ago, right?

Err. I am almost certain that the return value of GetProcessMemory isn't
a reliable way to verify success. Its final parameter is. Or at least
that is my experience. Of course, MSDN docs don't mention this.

I tested this on ARM and on i386. Real devices, not emulators. The ARM
implementation worked as it was, and continued to work after my fix. The
i386 version didn't work before, does work after the fix.

Is this sufficient explanation?

	Danny

-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Tuesday 30 June 2009 22:07:19, Danny Backx wrote:
> 
> > Is the rest of my patch acceptable or are there things I need to
> > address ?
> >

Did you get to confirm what really that ERROR_PIPE_NOT_CONNECTED
is about?

 http://sourceware.org/ml/gdb-patches/2009-06/msg00373.html

Could you post an updated, cleaned up patch, without any extra
unnecessary bits removed, along with change log entry, using
'cvs diff -up'?

And you never did explain what was broken with get_image_name
that you had to fix, as I asked $n emails ago, right?

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

Ping ?

	Danny

On Sun, 2009-06-21 at 11:50 +0200, Danny Backx wrote:
> On Sun, 2009-06-14 at 15:23 +0100, Pedro Alves wrote:
> [omitted discussion between Pedro and me about what to do with setjmp]
> > Read the code again.  It is used in more cases than just exiting.  In
> > any case, this would be going too far.  Replacements or an alternative
> > similar mechanism is in order.  
> 
> I've borrowed setjmp/longjmp assembler code from newlib and copied them
> into mingw/mingwex. So I was now able to compile gdbserver without the
> #if 0 lines to remove its calls to setjmp or longjmp .
> 
> Is the rest of my patch acceptable or are there things I need to
> address ?
> 
> Once this gdbserver stuff is done, I'll come up with the gdb part of my
> work.
> 
> 	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sun, 2009-06-14 at 15:23 +0100, Pedro Alves wrote:
[omitted discussion between Pedro and me about what to do with setjmp]
> Read the code again.  It is used in more cases than just exiting.  In
> any case, this would be going too far.  Replacements or an alternative
> similar mechanism is in order.  

I've borrowed setjmp/longjmp assembler code from newlib and copied them
into mingw/mingwex. So I was now able to compile gdbserver without the
#if 0 lines to remove its calls to setjmp or longjmp .

Is the rest of my patch acceptable or are there things I need to
address ?

Once this gdbserver stuff is done, I'll come up with the gdb part of my
work.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 14 June 2009 10:48:13, Danny Backx wrote:
> On Sun, 2009-06-14 at 10:47 +0200, Danny Backx wrote:
> > On Sat, 2009-06-13 at 19:05 +0100, Pedro Alves wrote:
> > > > 2. Handle a case where the inferior refuses to start. See the
> > > >    code after WaitForDebugEvent in win32-low.c .
> > > 
> > > This is quite mystifying.  Do you have more details on this?
> > 
> > Not yet, but as I said in the other message, the a simple setjmp demo
> > application is an example. Other cases I've seen were with invalid
> > requirements to a DLL (use of an API that was not in the DLL). On a
> > PocketPC, this causes a dialog to pop up. On Windows Embedded CE the
> > application appears just not to start up.

Huh, it has nothing to do with GUIs and popups.  The fact that this
doesn't result in CreateProcess returning ERROR_BAD_EXE_FORMAT is what
is quite mystifying.  Did you find any reference to this
ERROR_PIPE_NOT_CONNECTED case anywhere else?  So this supposedly
happens when the debug api is reporting the initial list of loaded
dlls?  Here's a test you could make to make it a bit clearer what
is happening:  make a simple test app that doesn't reference any
non-existing system function.  Make a couple of simple dlls, in which
one of them calls setjmp or whatever other function non-exiting
function.  Make the app link to these dlls.  Load it under gdbserver.
Check if WaitForDebugEvent reports any events, like loading coredll.dll
before reporting that ERROR_PIPE_NOT_CONNECTED.  I would guess from
your description that WaitForDebugEvent would fail when the faulty
dll is loaded.  BTW, trying the load the faulty dll dynamicaly
with LoadLibrary would hopefully fail gracefully, otherwise, you
may have found a kernel bug...

> > 
> > > > 3. Setjmp won't work on this platform, I've "#if 0"-ed it out.
> > > >    Clearly not the right solution. Comment please.
> > > 
> > > What does "won't work on this platform" mean?  If longjmp
> > > doesn't work, then you probably have a bug in your
> > > headers or import libs?  I'd suspect _JBLEN in the
> > > mingw headers.


> > I've been thinking about that too, I'll try to debug this but that's
> > hard with no debugger.

You *do* have a debugger.  You can debug gdbserver with gdbserver itself.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 14 June 2009 10:48:13, Danny Backx wrote:

> > > > 3. Setjmp won't work on this platform, I've "#if 0"-ed it out.
> > > >    Clearly not the right solution. Comment please.
> > > 
> > > What does "won't work on this platform" mean?  If longjmp
> > > doesn't work, then you probably have a bug in your
> > > headers or import libs?  I'd suspect _JBLEN in the
> > > mingw headers.
> > 
> > I've been thinking about that too, I'll try to debug this but that's
> > hard with no debugger.
> 
> The cause turns out to be the obvious one, if you read the above well.
> The setjmp call is documented on MSDN but my experiments show that the
> coredll.dll doesn't offer it on the platform I'm using.

Urgh.   MSFT claims that setjmp/longjmp is deprecated and that we
should use SEH instead... sigh.  This goes into the same dumb ideas
bucket as removing errno and any notion of cwd in Windows CE.  But it
may we be in this case that this is the manufacturer of the device
stripping away optional functions that they think are unnecessary for
their product.  Or this is a Windows Embedded profile thing?  Is there
any Win32 API function similar to setjmp/longjmp we could use instead?  Is
there any MSFT or vendor SDK for your device?  Could you get the list of
functions exported by coredll.dll and friends and send them
privately to me?

> - remove setjmp/longjmp and just exit in that one place where longjmp
>   is called
> - improve the code so errors are always treated well without relying
>   on longjmp
> 
> The last one is not something I'd like to start. My current code
> basically implements the second one, I'm not sure what would be going
> wrong if this would be implemented generally.

Read the code again.  It is used in more cases than just exiting.  In
any case, this would be going too far.  Replacements or an alternative
similar mechanism is in order.  Aren't you using the sjlj c++ exceptions
model?  If so, you'll need to write setjmp/longjmp replacements
anyway.  Maybe add them to libmingwex.a, and call them
mingw32ce_setjmp|mingw32ce_longjmp or something.  Or maybe there's
some import library magic we could use to pick up replacements
automaticaly.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 14 June 2009 09:47:05, Danny Backx wrote:
> > IIRC, psapi.dll does exist on WinCE, but it is
> > not bundled with the OS usually.  If I'm not confusing
> > it with some other dll, I think I have it for ARM, gotten
> > from some MSFT redistributable, and it does work.
> 
> It isn't on the Windows Embedded for x86 that I'm using. But the
> gdbserver code appears to be well written : it silently falls back to
> something that does work.
> 
> I've not messed with that code because "if it ain't broke, don't fix
> it". I could have surrounded it with #ifndef _WIN32_WCE but I didn't see
> the point.

I've no idea what you're talking about here, or even why
we talking about psapi.dll anyway.  The only hunks in your
patches touching psapi related things are:


-  if (!load_psapi ())
+  if (!load_psapi ()) {
     goto failed;
+  }

and...

+  /* Note : no psapi.dll on CE, fall back to get_image_name below. */
   if (!psapi_get_dll_name ((DWORD) event->lpBaseOfDll, dll_buf))


The fact that psapi is dynamicaly loaded with LoadLibrary
was exactly so that gdbserver falls back to other mechanisms.
Even on desktop Windows it isn't garanteed that psapi.dll will be
around.  Please drop these spurious hunks from the patch.

BTW, 

-  if (!load_psapi ())
+  if (!load_psapi ()) {
     goto failed;
+  }

The coding conventions states to not surround a single statement
with curly braces, so that original version would be the correct
form --- your patch would make it wrong.


+  /* Note : no psapi.dll on CE, fall back to get_image_name below. */

Double spaces after period, no space before ':'.  This would be:

  /* Note: no psapi.dll on CE, fall back to get_image_name below.  */

But as I said, this hunk is unnecessary.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Sunday 14 June 2009 09:34:41, Danny Backx wrote:

> Your comment about I need to address the setjmp issue is correct. I need
> to find the cause though. This part of the patch certainly doesn't
> belong in gdb's sources. 

> But it's still in discussion phase, isn't it ? 

It sure is.

> More comments ? Should the INT3 comment go away ? I feel that the code
> is under-commented at times, but this one may be somewhat overdone :-)

Yes.  int3 is the standard x86 breakpoint op.  It is the same on
Windows9x/NT, on linux (see linux-x86-low.c), and pretty much
every x86 OS.  win32-i386-low.c just didn't register a breakpoint 
instruction because Windodws gdbserver itself didn't need to insert
breakpoints (gdb takes care of regular user breakpoints by issuing
regular memory reads/writes)  --- on desktop Windows, the OS always
reports a magic "initial breakpoint hit" after reporting all threads and
dlls loaded in the process.  On Windows CE however, we fake it ourselves.
That's this code in win32-low.c:

...
#ifdef _WIN32_WCE
      if (!attaching)
	{
	  /* Windows CE doesn't set the initial breakpoint
	     automatically like the desktop versions of Windows do.
	     We add it explicitly here.	 It will be removed as soon as
	     it is hit.	 */
	  set_breakpoint_at ((CORE_ADDR) (long) current_event.u
			     .CreateProcessInfo.lpStartAddress,
			     auto_delete_breakpoint);
	}
#endif

...

#ifdef _WIN32_WCE
      /* Remove the initial breakpoint.  */
      check_breakpoints ((CORE_ADDR) (long) current_event
			 .u.Exception.ExceptionRecord.ExceptionAddress);
#endif

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sun, 2009-06-14 at 10:47 +0200, Danny Backx wrote:
> On Sat, 2009-06-13 at 19:05 +0100, Pedro Alves wrote:
> > > 2. Handle a case where the inferior refuses to start. See the
> > >    code after WaitForDebugEvent in win32-low.c .
> > 
> > This is quite mystifying.  Do you have more details on this?
> 
> Not yet, but as I said in the other message, the a simple setjmp demo
> application is an example. Other cases I've seen were with invalid
> requirements to a DLL (use of an API that was not in the DLL). On a
> PocketPC, this causes a dialog to pop up. On Windows Embedded CE the
> application appears just not to start up.
> 
> > > 3. Setjmp won't work on this platform, I've "#if 0"-ed it out.
> > >    Clearly not the right solution. Comment please.
> > 
> > What does "won't work on this platform" mean?  If longjmp
> > doesn't work, then you probably have a bug in your
> > headers or import libs?  I'd suspect _JBLEN in the
> > mingw headers.
> 
> I've been thinking about that too, I'll try to debug this but that's
> hard with no debugger.

The cause turns out to be the obvious one, if you read the above well.
The setjmp call is documented on MSDN but my experiments show that the
coredll.dll doesn't offer it on the platform I'm using.

Therefore, an application that tries to fetch it from the DLL will
simply not start up.

I've looked at the gdbserver code. The only two reasons I see for using
setjmp/longjmp are :
- to call kill_inferior () if you're beyond some point
- to end up at the "restart:" point

I see three options :
- write a setjmp replacement, put it in utils.c, and leave the rest of
  the code as is.
- remove setjmp/longjmp and just exit in that one place where longjmp
  is called
- improve the code so errors are always treated well without relying
  on longjmp

The last one is not something I'd like to start. My current code
basically implements the second one, I'm not sure what would be going
wrong if this would be implemented generally.

A final note : detecting this situation automatically at configure time
is not obvious because of the cross-compile.

Opinions ?

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

On Sat, 2009-06-13 at 19:05 +0100, Pedro Alves wrote:
> > 2. Handle a case where the inferior refuses to start. See the
> >    code after WaitForDebugEvent in win32-low.c .
> 
> This is quite mystifying.  Do you have more details on this?

Not yet, but as I said in the other message, the a simple setjmp demo
application is an example. Other cases I've seen were with invalid
requirements to a DLL (use of an API that was not in the DLL). On a
PocketPC, this causes a dialog to pop up. On Windows Embedded CE the
application appears just not to start up.

> > 3. Setjmp won't work on this platform, I've "#if 0"-ed it out.
> >    Clearly not the right solution. Comment please.
> 
> What does "won't work on this platform" mean?  If longjmp
> doesn't work, then you probably have a bug in your
> headers or import libs?  I'd suspect _JBLEN in the
> mingw headers.

I've been thinking about that too, I'll try to debug this but that's
hard with no debugger.

> IIRC, psapi.dll does exist on WinCE, but it is
> not bundled with the OS usually.  If I'm not confusing
> it with some other dll, I think I have it for ARM, gotten
> from some MSFT redistributable, and it does work.

It isn't on the Windows Embedded for x86 that I'm using. But the
gdbserver code appears to be well written : it silently falls back to
something that does work.

I've not messed with that code because "if it ain't broke, don't fix
it". I could have surrounded it with #ifndef _WIN32_WCE but I didn't see
the point.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

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

This patch is w.r.t. current CVS, and also fixes a couple of places
where I messed up the indenting. Is that what you referred to with the
standards comment ? I'm trying, I sometimes miss things though :-)

Your comment about I need to address the setjmp issue is correct. I need
to find the cause though. This part of the patch certainly doesn't
belong in gdb's sources. But it's still in discussion phase, isn't it ?

More comments ? Should the INT3 comment go away ? I feel that the code
is under-commented at times, but this one may be somewhat overdone :-)

	Danny

On Sat, 2009-06-13 at 19:08 +0100, Pedro Alves wrote:
> One last item:  Please do post patches against current
> CVS.  A lot has changed since 6.8, although I don't think
> your changes are much affected.
> 
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

[-- Attachment #2: gdbserver-diffs --]
[-- Type: text/x-patch, Size: 8167 bytes --]

Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.97
diff -u -r1.97 server.c
--- server.c	24 May 2009 21:06:53 -0000	1.97
+++ server.c	14 Jun 2009 08:29:23 -0000
@@ -1995,11 +1995,13 @@
       continue;
     }
 
+#if 0
   if (setjmp (toplevel))
     {
       fprintf (stderr, "Exiting\n");
       exit (1);
     }
+#endif
 
   port = *next_arg;
   next_arg++;
@@ -2077,11 +2079,13 @@
      shared library event" notice on gdb side.  */
   dlls_changed = 0;
 
+#if 0
   if (setjmp (toplevel))
     {
       detach_or_kill_for_exit ();
       exit (1);
     }
+#endif
 
   if (last_status.kind == TARGET_WAITKIND_EXITED
       || last_status.kind == TARGET_WAITKIND_SIGNALLED)
@@ -2103,6 +2107,7 @@
 
       remote_open (port);
 
+#if 0
       if (setjmp (toplevel) != 0)
 	{
 	  /* An error occurred.  */
@@ -2112,6 +2117,7 @@
 	      putpkt (own_buf);
 	    }
 	}
+#endif
 
       /* Wait for events.  This will return when all event sources are
 	 removed from the event loop. */
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/utils.c,v
retrieving revision 1.18
diff -u -r1.18 utils.c
--- utils.c	19 Jan 2009 00:16:46 -0000	1.18
+++ utils.c	14 Jun 2009 08:29:23 -0000
@@ -139,7 +139,11 @@
   fflush (stdout);
   vfprintf (stderr, string, args);
   fprintf (stderr, "\n");
+#ifdef __MINGW32CE__
+  exit(1);
+#else
   longjmp (toplevel, 1);
+#endif
 }
 
 /* Print an error message and exit reporting failure.
Index: win32-i386-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-i386-low.c,v
retrieving revision 1.14
diff -u -r1.14 win32-i386-low.c
--- win32-i386-low.c	3 Jan 2009 05:57:57 -0000	1.14
+++ win32-i386-low.c	14 Jun 2009 08:29:23 -0000
@@ -39,16 +39,36 @@
   debug_registers_used = 0;
 }
 
+/*
+ * According to Mike Stall's .net debugging blog
+ * (http://blogs.msdn.com/jmstall/archive/2005/01/18/355697.aspx)
+ * the CONTEXT_EXTENDED_REGISTERS flag must be omitted if hardware doesn't
+ * support it. So I guess the only reasonable thing to do is just try.
+ */
 static void
 i386_get_thread_context (win32_thread_info *th, DEBUG_EVENT* current_event)
 {
-  th->context.ContextFlags = \
-    CONTEXT_FULL | \
-    CONTEXT_FLOATING_POINT | \
-    CONTEXT_EXTENDED_REGISTERS | \
+  /* try all flags */
+  th->context.ContextFlags =
+    CONTEXT_FULL |
+    CONTEXT_FLOATING_POINT |
+    CONTEXT_EXTENDED_REGISTERS |
     CONTEXT_DEBUG_REGISTERS;
 
-  GetThreadContext (th->h, &th->context);
+  if (GetThreadContext (th->h, &th->context) == 0) {
+    DWORD e = GetLastError();
+
+    if (e == ERROR_INVALID_PARAMETER) {
+      /* try limited set */
+      th->context.ContextFlags = CONTEXT_FULL |
+	CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS;
+      if (GetThreadContext (th->h, &th->context) == 0) {
+	DWORD e = GetLastError();
+	printf("GetThreadContext failure %d\n", e);
+	return;
+      }
+    }
+  }
 
   debug_registers_changed = 0;
 
@@ -193,6 +213,27 @@
   collect_register (r, context_offset);
 }
 
+/*
+ * The INT 3 instruction is traditionally used for x86 platform breakpointing.
+ * Microsoft also appears to use a DebugBreak function, which probably does the same.
+ * Gas translates "int $3" (or "int3") to a one-byte instruction : 0xCC .
+ *
+ * From Wikipedia :
+ *
+ * The INT 3 instruction is defined for use by debuggers to temporarily replace
+ * an instruction in a running program, in order to set a breakpoint. Other INT
+ * instructions are encoded using two bytes. This makes them unsuitable for use
+ * in patching instructions (which can be one byte long).
+ *
+ * The opcode for INT 3 is 0xCC, as opposite from the opcode for INT immediate,
+ * which is 0xCD imm8. According to Intel documentation: "Intel and Microsoft
+ * assemblers will not generate the CD03 opcode from any mnemonic" and 0xCC
+ * has some special features, which are not shared by "the normal 2-byte
+ * opcode for INT 3 (CD03)" [IA-32 Arch. Software Developer’s Manual. Vol. 2A]
+ */
+static const unsigned char i386_wince_breakpoint = 0xCC;
+#define i386_wince_breakpoint_len 1
+
 struct win32_target_ops the_low_target = {
   init_registers_i386,
   sizeof (mappings) / sizeof (mappings[0]),
@@ -203,6 +244,6 @@
   i386_fetch_inferior_register,
   i386_store_inferior_register,
   i386_single_step,
-  NULL, /* breakpoint */
-  0, /* breakpoint_len */
+  &i386_wince_breakpoint, /* breakpoint */
+  i386_wince_breakpoint_len, /* breakpoint_len */
 };
Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.35
diff -u -r1.35 win32-low.c
--- win32-low.c	1 Apr 2009 22:50:24 -0000	1.35
+++ win32-low.c	14 Jun 2009 08:29:24 -0000
@@ -873,14 +873,19 @@
   loaded_dll (buf2, load_addr);
 }
 
+/*
+ * Warning : some parts of this function rely on sizeof(WCHAR) == 2
+ */
 static char *
 get_image_name (HANDLE h, void *address, int unicode)
 {
-  static char buf[(2 * MAX_PATH) + 1];
+  static char buf[(2 * MAX_PATH) + 1]; /* here */
   DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
   char *address_ptr;
+#ifndef _WIN32_WCE
   int len = 0;
-  char b[2];
+  char b[2]; /* here */
+#endif
   DWORD done;
 
   /* Attempt to read the name of the dll that was detected.
@@ -903,9 +908,28 @@
     return NULL;
 #endif
 
+#ifdef _WIN32_WCE
+  /* Always unicode */
+  /* Assume you can read it all in one go, or otherwise the done variable will
+   * tell you how far you've read.
+   */
+  WCHAR *wbuf = alloca ((MAX_PATH + 1) * size);
+  ReadProcessMemory (h, address_ptr, wbuf, MAX_PATH * size, &done);
+  if (done < 0 || done > MAX_PATH * size)
+	  buf[0] = '\0';
+  else {
+    int n;
+    n = wcstombs (buf, wbuf, done);
+    if (n == (size_t)-1)
+      buf[0] = '\0';
+    /* No need to address the length limit case of the wcstombs call,
+     * buf has been allocated large enough. */
+  }
+  return buf;
+#else
   /* Find the length of the string */
   while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
-	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
+	 && (b[0] != 0 || b[size - 1] != 0) && done == size) /* here */
     continue;
 
   if (!unicode)
@@ -915,11 +939,10 @@
       WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
       ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
 			 &done);
-
       WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
     }
-
   return buf;
+#endif
 }
 
 typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
@@ -969,8 +992,9 @@
   DWORD cbNeeded;
   BOOL ok;
 
-  if (!load_psapi ())
+  if (!load_psapi ()) {
     goto failed;
+  }
 
   cbNeeded = 0;
   ok = (*win32_EnumProcessModules) (current_process_handle,
@@ -1123,6 +1147,7 @@
   /* Windows does not report the image name of the dlls in the debug
      event on attaches.  We resort to iterating over the list of
      loaded dlls looking for a match by image base.  */
+  /* Note : no psapi.dll on CE, fall back to get_image_name below. */
   if (!psapi_get_dll_name ((DWORD) event->lpBaseOfDll, dll_buf))
     {
       if (!server_waiting)
@@ -1349,6 +1374,7 @@
 	 happen is the user will see a spurious breakpoint.  */
 
       current_event.dwDebugEventCode = 0;
+      OUTMSG2(("attaching: before WaitForDebugEvent\n"));
       if (!WaitForDebugEvent (&current_event, 0))
 	{
 	  OUTMSG2(("no attach events left\n"));
@@ -1365,7 +1391,20 @@
 	 interruption, but high enough so gdbserver doesn't become a
 	 bottleneck.  */
       if (!WaitForDebugEvent (&current_event, 250))
-	return 0;
+        {
+	  /*
+	   * Sometimes an application will just not start up.
+	   * Detect this here, return in such a way that the loop ends.
+	   */
+	  DWORD e = GetLastError();
+
+	  if (e == ERROR_PIPE_NOT_CONNECTED)
+	    {
+	      ourstatus->kind = TARGET_WAITKIND_EXITED;
+	      return 1;	/* break the loop in our caller */
+	    }
+   	  return 0;
+        }
     }
 
  gotevent:

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

One last item:  Please do post patches against current
CVS.  A lot has changed since 6.8, although I don't think
your changes are much affected.

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Pedro Alves]

On Saturday 13 June 2009 15:29:45, Danny Backx wrote:
> 1. Initialize the breakpoint structure to the right instruction.

This is fine.  All that commenting fuss about "int 3" is
really unnecessary though.  If it was any other instruction other than
0xcc I'd be surprised.

> 2. Handle a case where the inferior refuses to start. See the
>    code after WaitForDebugEvent in win32-low.c .

This is quite mystifying.  Do you have more details on this?

> 3. Setjmp won't work on this platform, I've "#if 0"-ed it out.
>    Clearly not the right solution. Comment please.

What does "won't work on this platform" mean?  If longjmp
doesn't work, then you probably have a bug in your
headers or import libs?  I'd suspect _JBLEN in the
mingw headers.

> 4. Handle the failing call to GetThreadContext.

Seems reasonable.

> 5. Read the DLL names in a way that works on x86.


Could you please post your next patches in unified format
please?  (cvs diff -up).

IIRC, psapi.dll does exist on WinCE, but it is
not bundled with the OS usually.  If I'm not confusing
it with some other dll, I think I have it for ARM, gotten
from some MSFT redistributable, and it does work.

Could you please take a look at the GNU coding
conventions, and reformat appropriately, please?

 http://www.gnu.org/prep/standards/standards.html#Writing-C

-- 
Pedro Alves

Re: Patch : gdbserver get_image_name on CE

[Danny Backx]

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

As I said gdbserver works with this patch, on CE on an x86 clone.

For the record, I'm testing this with a Via Eden ULV processor on
Windows Embedded CE 6.0 .

Several issues solved with this patch :
1. Initialize the breakpoint structure to the right instruction.
2. Handle a case where the inferior refuses to start. See the
   code after WaitForDebugEvent in win32-low.c .
3. Setjmp won't work on this platform, I've "#if 0"-ed it out.
   Clearly not the right solution. Comment please.
4. Handle the failing call to GetThreadContext.
5. Read the DLL names in a way that works on x86.

The patch I've attached is relative to gdb-6.8 .
The configure.srv bit in the attachment was sent (by me) and committed
(by Pedro) earlier, don't pay attention to it.
The patch for #5 was also recently sent by me, probably not committed
yet.

After comments on this, I'll transform this into a real patch.

Note that gdb also requires a bit of work, I've not polluted this patch
with that. I have it working too.

	Danny


-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

[-- Attachment #2: gdbserver-diffs --]
[-- Type: text/x-patch, Size: 11303 bytes --]

diff -c /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/configure.srv ./configure.srv
*** /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/configure.srv	2008-02-11 23:00:31.000000000 +0100
--- ./configure.srv	2009-04-17 19:49:46.000000000 +0200
***************
*** 65,70 ****
--- 65,79 ----
  			srv_linux_regsets=yes
  			srv_linux_thread_db=yes
  			;;
+   i[34567]86-*-mingw*ce*)
+ 			srv_regobj=reg-i386.o
+ 			srv_tgtobj="win32-low.o win32-i386-low.o"
+ 			srv_tgtobj="${srv_tgtobj} wincecompat.o"
+ 			# hostio_last_error implementation is in win32-low.c
+ 			srv_hostio_err_objs=""
+ 			srv_mingw=yes
+ 			srv_mingwce=yes
+ 			;;
    i[34567]86-*-mingw*)	srv_regobj=reg-i386.o
  			srv_tgtobj="win32-low.o win32-i386-low.o"
  			srv_mingw=yes
diff -c /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/server.c ./server.c
*** /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/server.c	2008-02-19 22:36:54.000000000 +0100
--- ./server.c	2009-06-13 11:57:58.000000000 +0200
***************
*** 1061,1071 ****
--- 1061,1073 ----
        continue;
      }
  
+ #if 0
    if (setjmp (toplevel))
      {
        fprintf (stderr, "Exiting\n");
        exit (1);
      }
+ #endif
  
    port = *next_arg;
    next_arg++;
***************
*** 1141,1153 ****
       shared library event" notice on gdb side.  */
    dlls_changed = 0;
  
    if (setjmp (toplevel))
      {
        fprintf (stderr, "Killing inferior\n");
        kill_inferior ();
        exit (1);
      }
! 
    if (status == 'W' || status == 'X')
      was_running = 0;
    else
--- 1143,1156 ----
       shared library event" notice on gdb side.  */
    dlls_changed = 0;
  
+ #if 0
    if (setjmp (toplevel))
      {
        fprintf (stderr, "Killing inferior\n");
        kill_inferior ();
        exit (1);
      }
! #endif
    if (status == 'W' || status == 'X')
      was_running = 0;
    else
***************
*** 1164,1169 ****
--- 1167,1173 ----
        remote_open (port);
  
      restart:
+ #if 0
        if (setjmp (toplevel) != 0)
  	{
  	  /* An error occurred.  */
***************
*** 1173,1178 ****
--- 1177,1183 ----
  	      putpkt (own_buf);
  	    }
  	}
+ #endif
  
        disable_async_io ();
        while (!exit_requested)
Only in .: spu-low.c~
diff -c /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/utils.c ./utils.c
*** /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/utils.c	2008-01-01 23:53:14.000000000 +0100
--- ./utils.c	2009-05-31 15:38:07.000000000 +0200
***************
*** 65,71 ****
--- 65,75 ----
    fflush (stdout);
    vfprintf (stderr, string, args);
    fprintf (stderr, "\n");
+ #ifdef __MINGW32CE__
+   exit(1);
+ #else
    longjmp (toplevel, 1);
+ #endif
  }
  
  /* Print an error message and exit reporting failure.
diff -c /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/win32-i386-low.c ./win32-i386-low.c
*** /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/win32-i386-low.c	2008-01-01 23:53:14.000000000 +0100
--- ./win32-i386-low.c	2009-06-13 11:50:19.000000000 +0200
***************
*** 36,51 ****
    debug_registers_used = 0;
  }
  
  static void
  i386_get_thread_context (win32_thread_info *th, DEBUG_EVENT* current_event)
  {
!   th->context.ContextFlags = \
!     CONTEXT_FULL | \
!     CONTEXT_FLOATING_POINT | \
!     CONTEXT_EXTENDED_REGISTERS | \
      CONTEXT_DEBUG_REGISTERS;
  
!   GetThreadContext (th->h, &th->context);
  
    debug_registers_changed = 0;
  
--- 36,71 ----
    debug_registers_used = 0;
  }
  
+ /*
+  * According to Mike Stall's .net debugging blog
+  * (http://blogs.msdn.com/jmstall/archive/2005/01/18/355697.aspx)
+  * the CONTEXT_EXTENDED_REGISTERS flag must be omitted if hardware doesn't
+  * support it. So I guess the only reasonable thing to do is just try.
+  */
  static void
  i386_get_thread_context (win32_thread_info *th, DEBUG_EVENT* current_event)
  {
!   /* try all flags */
!   th->context.ContextFlags =
!     CONTEXT_FULL |
!     CONTEXT_FLOATING_POINT |
!     CONTEXT_EXTENDED_REGISTERS |
      CONTEXT_DEBUG_REGISTERS;
  
!   if (GetThreadContext (th->h, &th->context) == 0) {
!     DWORD e = GetLastError();
! 
!     if (e == ERROR_INVALID_PARAMETER) {
!       /* try limited set */
!       th->context.ContextFlags = CONTEXT_FULL |
! 	CONTEXT_FLOATING_POINT | CONTEXT_DEBUG_REGISTERS;
!       if (GetThreadContext (th->h, &th->context) == 0) {
! 	DWORD e = GetLastError();
! 	printf("GetThreadContext failure %d\n", e);
! 	return;
!       }
!     }
!   }
  
    debug_registers_changed = 0;
  
***************
*** 190,195 ****
--- 210,236 ----
    collect_register (r, context_offset);
  }
  
+ /*
+  * The INT 3 instruction is traditionally used for x86 platform breakpointing.
+  * Microsoft also appears to use a DebugBreak function, which probably does the same.
+  * Gas translates "int $3" (or "int3") to a one-byte instruction : 0xCC .
+  *
+  * From Wikipedia :
+  *
+  * The INT 3 instruction is defined for use by debuggers to temporarily replace
+  * an instruction in a running program, in order to set a breakpoint. Other INT
+  * instructions are encoded using two bytes. This makes them unsuitable for use
+  * in patching instructions (which can be one byte long).
+  *
+  * The opcode for INT 3 is 0xCC, as opposite from the opcode for INT immediate,
+  * which is 0xCD imm8. According to Intel documentation: "Intel and Microsoft
+  * assemblers will not generate the CD03 opcode from any mnemonic" and 0xCC
+  * has some special features, which are not shared by "the normal 2-byte
+  * opcode for INT 3 (CD03)" [IA-32 Arch. Software Developer’s Manual. Vol. 2A]
+  */
+ static const unsigned char i386_wince_breakpoint = 0xCC;
+ #define i386_wince_breakpoint_len 1
+ 
  struct win32_target_ops the_low_target = {
    sizeof (mappings) / sizeof (mappings[0]),
    i386_initial_stuff,
***************
*** 199,205 ****
    i386_fetch_inferior_register,
    i386_store_inferior_register,
    i386_single_step,
!   NULL, /* breakpoint */
!   0, /* breakpoint_len */
    "i386" /* arch_string */
  };
--- 240,246 ----
    i386_fetch_inferior_register,
    i386_store_inferior_register,
    i386_single_step,
!   &i386_wince_breakpoint, /* breakpoint */
!   i386_wince_breakpoint_len, /* breakpoint_len */
    "i386" /* arch_string */
  };
diff -c /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/win32-low.c ./win32-low.c
*** /home/danny/src/gdb/gdb/gdb-6.8.orig/gdb/gdbserver/win32-low.c	2008-02-14 23:41:39.000000000 +0100
--- ./win32-low.c	2009-06-13 16:18:48.000000000 +0200
***************
*** 894,907 ****
    loaded_dll (buf2, load_addr);
  }
  
  static char *
  get_image_name (HANDLE h, void *address, int unicode)
  {
!   static char buf[(2 * MAX_PATH) + 1];
    DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
    char *address_ptr;
    int len = 0;
!   char b[2];
    DWORD done;
  
    /* Attempt to read the name of the dll that was detected.
--- 894,912 ----
    loaded_dll (buf2, load_addr);
  }
  
+ /*
+  * Warning : some parts of this function rely on sizeof(WCHAR) == 2
+  */
  static char *
  get_image_name (HANDLE h, void *address, int unicode)
  {
!   static char buf[(2 * MAX_PATH) + 1]; /* here */
    DWORD size = unicode ? sizeof (WCHAR) : sizeof (char);
    char *address_ptr;
+ #ifndef _WIN32_WCE
    int len = 0;
!   char b[2]; /* here */
! #endif
    DWORD done;
  
    /* Attempt to read the name of the dll that was detected.
***************
*** 924,932 ****
      return NULL;
  #endif
  
    /* Find the length of the string */
    while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
! 	 && (b[0] != 0 || b[size - 1] != 0) && done == size)
      continue;
  
    if (!unicode)
--- 929,956 ----
      return NULL;
  #endif
  
+ #ifdef _WIN32_WCE
+   /* Always unicode */
+   /* Assume you can read it all in one go, or otherwise the done variable will
+    * tell you how far you've read.
+    */
+   WCHAR *wbuf = alloca ((MAX_PATH + 1) * size);
+   ReadProcessMemory (h, address_ptr, wbuf, MAX_PATH * size, &done);
+   if (done < 0 || done > MAX_PATH * size)
+ 	  buf[0] = '\0';
+   else {
+     int n;
+     n = wcstombs (buf, wbuf, done);
+     if (n == (size_t)-1)
+       buf[0] = '\0';
+     /* No need to address the length limit case of the wcstombs call,
+      * buf has been allocated large enough. */
+   }
+   return buf;
+ #else
    /* Find the length of the string */
    while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done)
! 	 && (b[0] != 0 || b[size - 1] != 0) && done == size) /* here */
      continue;
  
    if (!unicode)
***************
*** 936,946 ****
        WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
        ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
  			 &done);
- 
        WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
      }
- 
    return buf;
  }
  
  typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
--- 960,969 ----
        WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR));
        ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR),
  			 &done);
        WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0);
      }
    return buf;
+ #endif
  }
  
  typedef BOOL (WINAPI *winapi_EnumProcessModules) (HANDLE, HMODULE *,
***************
*** 990,997 ****
    DWORD cbNeeded;
    BOOL ok;
  
!   if (!load_psapi ())
      goto failed;
  
    cbNeeded = 0;
    ok = (*win32_EnumProcessModules) (current_process_handle,
--- 1013,1021 ----
    DWORD cbNeeded;
    BOOL ok;
  
!   if (!load_psapi ()) {
      goto failed;
+   }
  
    cbNeeded = 0;
    ok = (*win32_EnumProcessModules) (current_process_handle,
***************
*** 1144,1149 ****
--- 1168,1174 ----
    /* Windows does not report the image name of the dlls in the debug
       event on attaches.  We resort to iterating over the list of
       loaded dlls looking for a match by image base.  */
+   /* Note : no psapi.dll on CE, fall back to get_image_name below. */
    if (!psapi_get_dll_name ((DWORD) event->lpBaseOfDll, dll_buf))
      {
        if (!server_waiting)
***************
*** 1368,1373 ****
--- 1393,1399 ----
   	 happen is the user will see a spurious breakpoint.  */
  
        current_event.dwDebugEventCode = 0;
+       OUTMSG2(("attaching: before WaitForDebugEvent\n"));
        if (!WaitForDebugEvent (&current_event, 0))
   	{
   	  OUTMSG2(("no attach events left\n"));
***************
*** 1383,1390 ****
        /* Keep the wait time low enough for confortable remote
   	 interruption, but high enough so gdbserver doesn't become a
   	 bottleneck.  */
!       if (!WaitForDebugEvent (&current_event, 250))
   	return 0;
      }
  
   gotevent:
--- 1409,1427 ----
        /* Keep the wait time low enough for confortable remote
   	 interruption, but high enough so gdbserver doesn't become a
   	 bottleneck.  */
!       if (!WaitForDebugEvent (&current_event, 250)) {
! 	/*
! 	 * Sometimes an application will just not start up.
! 	 * Detect this here, return in such a way that the loop ends.
! 	 */
! 	DWORD e = GetLastError();
! 
! 	if (e == ERROR_PIPE_NOT_CONNECTED) {
! 	  ourstatus->kind = TARGET_WAITKIND_EXITED;
! 	  return 1;	/* break the loop in our caller */
! 	}
   	return 0;
+       }
      }
  
   gotevent: