[patch]: Replace stryoul call to fetch address

[patch]: Replace stryoul call to fetch address

[Corinna Vinschen]

Hi,

here's a small patch which will help along building GDB on the (not
yet finished) x86_64 Cygwin.

Mostly these are just a few casts, and tweaks to the printf format
strings, so that the argument and the format matches all three cases:

- i686
- x86_64 LLP (Mingw, native Windows)
- x86_64 LP  (Cygwin)

The most important part of the patch is in handle_output_debug_string,
though.  The address for the context information is read from the string
using the strtoul function.  This works fine for a 32 bit GDB, and it
also works fine for a 64 bit Cygwin GDB.  It does not work for a 64 bit
Mingw GDB debugging a 64 bit Cygwin application, though.  Therefore I
replaced the strtoul with a call to string_to_core_addr.

Apart from that there's more required to get GDB working on Cygwin,
apparently, but I thought the below patch is simple enough to go in
already.

Ok to apply?


Thanks,
Corinna


	* windows-nat.c: Throughout, fix format strings and casts of
	printf-like functions to avoid type related warnings on all
	platforms.
	(handle_output_debug_string): Fetch context information address
	from debug string using string_to_core_addr.


Index: windows-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/windows-nat.c,v
retrieving revision 1.240
diff -u -p -r1.240 windows-nat.c
--- windows-nat.c	19 Feb 2013 15:46:32 -0000	1.240
+++ windows-nat.c	27 Feb 2013 16:42:42 -0000
@@ -289,8 +289,8 @@ static void
 check (BOOL ok, const char *file, int line)
 {
   if (!ok)
-    printf_filtered ("error return %s:%d was %lu\n", file, line,
-		     GetLastError ());
+    printf_filtered ("error return %s:%d was %u\n", file, line,
+		     (unsigned) GetLastError ());
 }
 
 /* Find a thread record given a thread id.  If GET_CONTEXT is not 0,
@@ -311,7 +311,7 @@ thread_rec (DWORD id, int get_context)
 		if (SuspendThread (th->h) == (DWORD) -1)
 		  {
 		    DWORD err = GetLastError ();
-		    warning (_("SuspendThread failed. (winerr %d)"),
+		    warning (_("SuspendThread failed. (winerr %u)"),
 			     (int) err);
 		    return NULL;
 		  }
@@ -576,7 +576,8 @@ get_module_name (LPVOID base_address, ch
 	  len = GetModuleFileNameEx (current_process_handle,
 				      DllHandle[i], pathbuf, __PMAX);
 	  if (len == 0)
-	    error (_("Error getting dll name: %lu."), GetLastError ());
+	    error (_("Error getting dll name: %u."),
+		   (unsigned) GetLastError ());
 	  if (cygwin_conv_path (CCP_WIN_W_TO_POSIX, pathbuf, dll_name_ret,
 				__PMAX) < 0)
 	    error (_("Error converting dll name to POSIX: %d."), errno);
@@ -977,7 +978,7 @@ handle_output_debug_string (struct targe
 	  retval = strtoul (p, &p, 0);
 	  if (!retval)
 	    retval = main_thread_id;
-	  else if ((x = (LPCVOID) strtoul (p, &p, 0))
+	  else if ((x = (LPCVOID) string_to_core_addr (p))
 		   && ReadProcessMemory (current_process_handle, x,
 					 &saved_context,
 					 __COPY_CONTEXT_SIZE, &n)
@@ -1000,7 +1001,7 @@ display_selector (HANDLE thread, DWORD s
   if (GetThreadSelectorEntry (thread, sel, &info))
     {
       int base, limit;
-      printf_filtered ("0x%03lx: ", sel);
+      printf_filtered ("0x%03x: ", (unsigned) sel);
       if (!info.HighWord.Bits.Pres)
 	{
 	  puts_filtered ("Segment not present\n");
@@ -1064,7 +1065,7 @@ display_selector (HANDLE thread, DWORD s
       if (err == ERROR_NOT_SUPPORTED)
 	printf_filtered ("Function not supported\n");
       else
-	printf_filtered ("Invalid selector 0x%lx.\n",sel);
+	printf_filtered ("Invalid selector 0x%x.\n", (unsigned) sel);
       return 0;
     }
 }
@@ -1228,8 +1229,8 @@ handle_exception (struct target_waitstat
       /* Treat unhandled first chance exceptions specially.  */
       if (current_event.u.Exception.dwFirstChance)
 	return -1;
-      printf_unfiltered ("gdb: unknown target exception 0x%08lx at %s\n",
-	current_event.u.Exception.ExceptionRecord.ExceptionCode,
+      printf_unfiltered ("gdb: unknown target exception 0x%08x at %s\n",
+	(unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
 	host_address_to_string (
 	  current_event.u.Exception.ExceptionRecord.ExceptionAddress));
       ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
@@ -1249,8 +1250,9 @@ windows_continue (DWORD continue_status,
   thread_info *th;
   BOOL res;
 
-  DEBUG_EVENTS (("ContinueDebugEvent (cpid=%ld, ctid=%lx, %s);\n",
-		  current_event.dwProcessId, current_event.dwThreadId,
+  DEBUG_EVENTS (("ContinueDebugEvent (cpid=%d, ctid=%x, %s);\n",
+		  (unsigned) current_event.dwProcessId,
+		  (unsigned) current_event.dwThreadId,
 		  continue_status == DBG_CONTINUE ?
 		  "DBG_CONTINUE" : "DBG_EXCEPTION_NOT_HANDLED"));
 
@@ -1297,8 +1299,8 @@ fake_create_process (void)
     open_process_used = 1;
   else
     {
-      error (_("OpenProcess call failed, GetLastError = %lud"),
-       GetLastError ());
+      error (_("OpenProcess call failed, GetLastError = %u"),
+       (unsigned) GetLastError ());
       /*  We can not debug anything in that case.  */
     }
   main_thread_id = current_event.dwThreadId;
@@ -1457,7 +1459,7 @@ get_windows_debug_event (struct target_o
   switch (event_code)
     {
     case CREATE_THREAD_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "CREATE_THREAD_DEBUG_EVENT"));
@@ -1486,7 +1488,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case EXIT_THREAD_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "EXIT_THREAD_DEBUG_EVENT"));
@@ -1500,7 +1502,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case CREATE_PROCESS_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "CREATE_PROCESS_DEBUG_EVENT"));
@@ -1522,7 +1524,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case EXIT_PROCESS_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "EXIT_PROCESS_DEBUG_EVENT"));
@@ -1542,7 +1544,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case LOAD_DLL_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "LOAD_DLL_DEBUG_EVENT"));
@@ -1556,7 +1558,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case UNLOAD_DLL_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "UNLOAD_DLL_DEBUG_EVENT"));
@@ -1569,7 +1571,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case EXCEPTION_DEBUG_EVENT:
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "EXCEPTION_DEBUG_EVENT"));
@@ -1591,7 +1593,7 @@ get_windows_debug_event (struct target_o
       break;
 
     case OUTPUT_DEBUG_STRING_EVENT:	/* Message from the kernel.  */
-      DEBUG_EVENTS (("gdb: kernel event for pid=%d tid=%x code=%s)\n",
+      DEBUG_EVENTS (("gdb: kernel event for pid=%u tid=%x code=%s)\n",
 		     (unsigned) current_event.dwProcessId,
 		     (unsigned) current_event.dwThreadId,
 		     "OUTPUT_DEBUG_STRING_EVENT"));
@@ -1603,11 +1605,11 @@ get_windows_debug_event (struct target_o
     default:
       if (saw_create != 1)
 	break;
-      printf_unfiltered ("gdb: kernel event for pid=%ld tid=%ld\n",
-			 (DWORD) current_event.dwProcessId,
-			 (DWORD) current_event.dwThreadId);
-      printf_unfiltered ("                 unknown event code %ld\n",
-			 current_event.dwDebugEventCode);
+      printf_unfiltered ("gdb: kernel event for pid=%u tid=%x\n",
+			 (unsigned) current_event.dwProcessId,
+			 (unsigned) current_event.dwThreadId);
+      printf_unfiltered ("                 unknown event code %u\n",
+			 (unsigned) current_event.dwDebugEventCode);
       break;
     }
 
@@ -1865,8 +1867,8 @@ windows_detach (struct target_ops *ops, 
 
   if (!DebugActiveProcessStop (current_event.dwProcessId))
     {
-      error (_("Can't detach process %lu (error %lu)"),
-	     current_event.dwProcessId, GetLastError ());
+      error (_("Can't detach process %u (error %u)"),
+	     (unsigned) current_event.dwProcessId, (unsigned) GetLastError ());
       detached = 0;
     }
   DebugSetProcessKillOnExit (FALSE);
@@ -1876,8 +1878,8 @@ windows_detach (struct target_ops *ops, 
       char *exec_file = get_exec_file (0);
       if (exec_file == 0)
 	exec_file = "";
-      printf_unfiltered ("Detaching from program: %s, Pid %lu\n", exec_file,
-			 current_event.dwProcessId);
+      printf_unfiltered ("Detaching from program: %s, Pid %u\n", exec_file,
+			 (unsigned) current_event.dwProcessId);
       gdb_flush (gdb_stdout);
     }
 
@@ -2264,7 +2266,7 @@ windows_create_inferior (struct target_o
 #endif
 
   if (!ret)
-    error (_("Error creating process %s, (error %d)."),
+    error (_("Error creating process %s, (error %u)."),
 	   exec_file, (unsigned) GetLastError ());
 
   CloseHandle (pi.hThread);

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch]: Replace stryoul call to fetch address

[Pedro Alves]

On 02/27/2013 04:44 PM, Corinna Vinschen wrote:
> Hi,

Hi,

> 
> here's a small patch which will help along building GDB on the (not
> yet finished) x86_64 Cygwin.
> 
> Mostly these are just a few casts, and tweaks to the printf format
> strings, so that the argument and the format matches all three cases:
> 
> - i686
> - x86_64 LLP (Mingw, native Windows)
> - x86_64 LP  (Cygwin)
> 
> The most important part of the patch is in handle_output_debug_string,
> though.  The address for the context information is read from the string
> using the strtoul function.  This works fine for a 32 bit GDB, and it
> also works fine for a 64 bit Cygwin GDB.  It does not work for a 64 bit
> Mingw GDB debugging a 64 bit Cygwin application, though.  Therefore I
> replaced the strtoul with a call to string_to_core_addr.

IIRC, the matching Cygwin code that that special
Cygwin signals handling was never implemented, or it was disabled
on Cygwin, or some such, and that gdb bits is actually causing
trouble -- see http://sourceware.org/ml/gdb-patches/2013-02/msg00122.html.
We should just zap it all.

> Apart from that there's more required to get GDB working on Cygwin,
> apparently, but I thought the below patch is simple enough to go in
> already.

One thing that comes to mind is I think we'll need to have separate
mingw64/cygwin64 osabis.  Currently, mingw 32/64 use
GDB_OSABI_CYGWIN, and that limps along, but with LP vs LLP, that
won't work.  We'll need a way to distinguish Cygwin vs native Windows
binaries.  Probably by checking for cygwin.dll in the dll import list?

Another point where that'd be good is in step-over-longjmp support
(gdbarch_get_longjmp_target).  IIRC, the offset of PC within
the jmpbuf of msvcrt.dll is not the same as Cygwin's (32-bit;
dunno about 64-bit).

> 
> Ok to apply?

>  		    DWORD err = GetLastError ();
> -		    warning (_("SuspendThread failed. (winerr %d)"),
> +		    warning (_("SuspendThread failed. (winerr %u)"),
>  			     (int) err);

This one doesn't look right.  %d matches the cast to signed int.
I think you wanted the converse.  %u and (unsigned).

Otherwise looks fine to me.

-- 
Pedro Alves

Re: [patch]: Replace stryoul call to fetch address

[Corinna Vinschen]

On Feb 27 17:20, Pedro Alves wrote:
> On 02/27/2013 04:44 PM, Corinna Vinschen wrote:
> > Hi,
> 
> Hi,
> 
> > 
> > here's a small patch which will help along building GDB on the (not
> > yet finished) x86_64 Cygwin.
> > 
> > Mostly these are just a few casts, and tweaks to the printf format
> > strings, so that the argument and the format matches all three cases:
> > 
> > - i686
> > - x86_64 LLP (Mingw, native Windows)
> > - x86_64 LP  (Cygwin)
> > 
> > The most important part of the patch is in handle_output_debug_string,
> > though.  The address for the context information is read from the string
> > using the strtoul function.  This works fine for a 32 bit GDB, and it
> > also works fine for a 64 bit Cygwin GDB.  It does not work for a 64 bit
> > Mingw GDB debugging a 64 bit Cygwin application, though.  Therefore I
> > replaced the strtoul with a call to string_to_core_addr.
> 
> IIRC, the matching Cygwin code that that special
> Cygwin signals handling was never implemented, or it was disabled
> on Cygwin, or some such, and that gdb bits is actually causing
> trouble -- see http://sourceware.org/ml/gdb-patches/2013-02/msg00122.html.
> We should just zap it all.

I don't know exactly.  Probably cgf would be able to answer this
better than me.  In fact I have a SEGV right now which I still have
to investigate, but I'm working on GDB only as a side job.

> > Apart from that there's more required to get GDB working on Cygwin,
> > apparently, but I thought the below patch is simple enough to go in
> > already.
> 
> One thing that comes to mind is I think we'll need to have separate
> mingw64/cygwin64 osabis.  Currently, mingw 32/64 use
> GDB_OSABI_CYGWIN, and that limps along, but with LP vs LLP, that
> won't work.

Why not?  The only difference between the x86_64 Cygwin and Mingw ABI
is the sizeof long.  And that's noted in the dwarf debug info.  Apart
from a strange crash when trying to load stripped executables, I'm
using a x86_64 Mingw GDB to debug x86_64 Cygwin DLL and binaries.
I'm not sure this single difference justifies distinct OSABIs.

> We'll need a way to distinguish Cygwin vs native Windows
> binaries.  Probably by checking for cygwin.dll in the dll import list?

A check for cygwin1.dll is already done in windows_make_so.

> Another point where that'd be good is in step-over-longjmp support
> (gdbarch_get_longjmp_target).  IIRC, the offset of PC within
> the jmpbuf of msvcrt.dll is not the same as Cygwin's (32-bit;
> dunno about 64-bit).

Cygwin 64 bit uses the same jmpbuf layout as native Windows.  Only
two members are used differently, but those should not be used by
GDB anyway.

> > Ok to apply?
> 
> >  		    DWORD err = GetLastError ();
> > -		    warning (_("SuspendThread failed. (winerr %d)"),
> > +		    warning (_("SuspendThread failed. (winerr %u)"),
> >  			     (int) err);
> 
> This one doesn't look right.  %d matches the cast to signed int.
> I think you wanted the converse.  %u and (unsigned).

Sorry, I missed the int cast.  I planned to replace that with unsigned
as well, yes.

> Otherwise looks fine to me.

Ok, I apply it then with only the int changed to unsigned.


Thanks
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch]: Replace stryoul call to fetch address

[Pedro Alves]

On 02/27/2013 06:38 PM, Corinna Vinschen wrote:

>> IIRC, the matching Cygwin code that that special
>> Cygwin signals handling was never implemented, or it was disabled
>> on Cygwin, or some such, and that gdb bits is actually causing
>> trouble -- see http://sourceware.org/ml/gdb-patches/2013-02/msg00122.html.
>> We should just zap it all.
> 
> I don't know exactly.  Probably cgf would be able to answer this
> better than me.  

I think I recall some email from him long about about the
cygwin side of that, but he wasn't against removing these
bits of on the gdb end in that url above, so...

> In fact I have a SEGV right now which I still have
> to investigate, but I'm working on GDB only as a side job.

Sure.  I just meant to point out that that's exactly the code
you're touching.  ;-)

>> One thing that comes to mind is I think we'll need to have separate
>> mingw64/cygwin64 osabis.  Currently, mingw 32/64 use
>> GDB_OSABI_CYGWIN, and that limps along, but with LP vs LLP, that
>> won't work.
> 
> Why not?  The only difference between the x86_64 Cygwin and Mingw ABI
> is the sizeof long.  And that's noted in the dwarf debug info.

Not every use use of the target's "long" goes through the debug info.  I
see uses of  gdbarch_long_bit and builtin_long in the expression
machinery, for example in c-exp.y, for handling integer constants,
or in eval.c, for type promotion.  x64's long is fixed to 32-bit
in amd64_windows_init_abi.

> Apart
> from a strange crash when trying to load stripped executables, I'm
> using a x86_64 Mingw GDB to debug x86_64 Cygwin DLL and binaries.
> I'm not sure this single difference justifies distinct OSABIs.

Sure, we'll limp along.  But there are cases that bypass debug info.
A distinct OSABI seems like the proper mechanism to me.  Time
will tell.  ;-)

> 
>> We'll need a way to distinguish Cygwin vs native Windows
>> binaries.  Probably by checking for cygwin.dll in the dll import list?
> 
> A check for cygwin1.dll is already done in windows_make_so.

Yeah, though that's only after the dll is loaded.  I was thinking
of looking at import list, when the executable is loaded in gdb,
hence before the program is run.

> 
>> Another point where that'd be good is in step-over-longjmp support
>> (gdbarch_get_longjmp_target).  IIRC, the offset of PC within
>> the jmpbuf of msvcrt.dll is not the same as Cygwin's (32-bit;
>> dunno about 64-bit).
> 
> Cygwin 64 bit uses the same jmpbuf layout as native Windows.  Only
> two members are used differently, but those should not be used by
> GDB anyway.

OK.

> Ok, I apply it then with only the int changed to unsigned.

Thanks.

-- 
Pedro Alves

Re: [patch]: Replace stryoul call to fetch address

[Corinna Vinschen]

On Feb 27 19:14, Pedro Alves wrote:
> On 02/27/2013 06:38 PM, Corinna Vinschen wrote:
> 
> >> IIRC, the matching Cygwin code that that special
> >> Cygwin signals handling was never implemented, or it was disabled
> >> on Cygwin, or some such, and that gdb bits is actually causing
> >> trouble -- see http://sourceware.org/ml/gdb-patches/2013-02/msg00122.html.
> >> We should just zap it all.
> > 
> > I don't know exactly.  Probably cgf would be able to answer this
> > better than me.  
> 
> I think I recall some email from him long about about the
> cygwin side of that, but he wasn't against removing these
> bits of on the gdb end in that url above, so...

Chris, ping?

> > In fact I have a SEGV right now which I still have
> > to investigate, but I'm working on GDB only as a side job.
> 
> Sure.  I just meant to point out that that's exactly the code
> you're touching.  ;-)

The SEGV occurs in exception.c, function throw_exception, though.
The `*current_catcher->exception = exception;' assignment crashes
because current_catcher->exception is NULL.  I don't understand yet
why it's NULL, and why the throw_exception function doesn't test
this before trying to write *current_catcher->exception.

> >> One thing that comes to mind is I think we'll need to have separate
> >> mingw64/cygwin64 osabis.  Currently, mingw 32/64 use
> >> GDB_OSABI_CYGWIN, and that limps along, but with LP vs LLP, that
> >> won't work.
> > 
> > Why not?  The only difference between the x86_64 Cygwin and Mingw ABI
> > is the sizeof long.  And that's noted in the dwarf debug info.
> 
> Not every use use of the target's "long" goes through the debug info.  I
> see uses of  gdbarch_long_bit and builtin_long in the expression
> machinery, for example in c-exp.y, for handling integer constants,
> or in eval.c, for type promotion.  x64's long is fixed to 32-bit
> in amd64_windows_init_abi.

Oh, hmm.  I didn't notice that.  So, well, maybe...

> > Apart
> > from a strange crash when trying to load stripped executables, I'm
> > using a x86_64 Mingw GDB to debug x86_64 Cygwin DLL and binaries.
> > I'm not sure this single difference justifies distinct OSABIs.
> 
> Sure, we'll limp along.  But there are cases that bypass debug info.
> A distinct OSABI seems like the proper mechanism to me.  Time
> will tell.  ;-)

...you're right.  I just don't know if I'm really the right person
to do that.

> [...]
> > Ok, I apply it then with only the int changed to unsigned.
> 
> Thanks.

Applied.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Corinna Vinschen]

On Feb 27 20:42, Corinna Vinschen wrote:
> > > Ok, I apply it then with only the int changed to unsigned.
> > 
> > Thanks.
> 
> Applied.

Here's an equivalent patch to gdbserver/win32-low.c.  Ok to apply?


Thanks,
Corinna


	* win32-low.c: Throughout, fix format strings and casts of
	printf-like functions to avoid type related warnings on all
	platforms.
	(get_child_debug_event): Print dwDebugEventCode as hex since
	that's how it's usually documented.


Index: win32-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
retrieving revision 1.62
diff -u -p -r1.62 win32-low.c
--- win32-low.c	1 Jan 2013 06:33:00 -0000	1.62
+++ win32-low.c	27 Feb 2013 19:47:47 -0000
@@ -448,7 +448,7 @@ strwinerror (DWORD error)
       LocalFree (msgbuf);
     }
   else
-    sprintf (buf, "unknown win32 error (%ld)", error);
+    sprintf (buf, "unknown win32 error (%u)", (unsigned) error);
 
   SetLastError (lasterr);
   return buf;
@@ -1317,10 +1317,10 @@ handle_exception (struct target_waitstat
 	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
 	  return;
 	}
-      OUTMSG2 (("gdbserver: unknown target exception 0x%08lx at 0x%s",
-		current_event.u.Exception.ExceptionRecord.ExceptionCode,
-		phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
-		ExceptionAddress, sizeof (uintptr_t))));
+      OUTMSG2 (("gdbserver: unknown target exception 0x%08x at 0x%s",
+	    (unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
+	    phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
+	    ExceptionAddress, sizeof (uintptr_t))));
       ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
       break;
     }
@@ -1452,7 +1452,7 @@ get_child_debug_event (struct target_wai
     {
     case CREATE_THREAD_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event CREATE_THREAD_DEBUG_EVENT "
-		"for pid=%d tid=%x)\n",
+		"for pid=%u tid=%x)\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
 
@@ -1465,7 +1465,7 @@ get_child_debug_event (struct target_wai
 
     case EXIT_THREAD_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event EXIT_THREAD_DEBUG_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       child_delete_thread (current_event.dwProcessId,
@@ -1476,7 +1476,7 @@ get_child_debug_event (struct target_wai
 
     case CREATE_PROCESS_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event CREATE_PROCESS_DEBUG_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       CloseHandle (current_event.u.CreateProcessInfo.hFile);
@@ -1510,7 +1510,7 @@ get_child_debug_event (struct target_wai
 
     case EXIT_PROCESS_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event EXIT_PROCESS_DEBUG_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       ourstatus->kind = TARGET_WAITKIND_EXITED;
@@ -1522,7 +1522,7 @@ get_child_debug_event (struct target_wai
 
     case LOAD_DLL_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event LOAD_DLL_DEBUG_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       CloseHandle (current_event.u.LoadDll.hFile);
@@ -1534,7 +1534,7 @@ get_child_debug_event (struct target_wai
 
     case UNLOAD_DLL_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event UNLOAD_DLL_DEBUG_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       handle_unload_dll ();
@@ -1544,7 +1544,7 @@ get_child_debug_event (struct target_wai
 
     case EXCEPTION_DEBUG_EVENT:
       OUTMSG2 (("gdbserver: kernel event EXCEPTION_DEBUG_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       handle_exception (ourstatus);
@@ -1553,7 +1553,7 @@ get_child_debug_event (struct target_wai
     case OUTPUT_DEBUG_STRING_EVENT:
       /* A message from the kernel (or Cygwin).  */
       OUTMSG2 (("gdbserver: kernel event OUTPUT_DEBUG_STRING_EVENT "
-		"for pid=%d tid=%x\n",
+		"for pid=%u tid=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId));
       handle_output_debug_string (ourstatus);
@@ -1561,10 +1561,10 @@ get_child_debug_event (struct target_wai
 
     default:
       OUTMSG2 (("gdbserver: kernel event unknown "
-		"for pid=%d tid=%x code=%ld\n",
+		"for pid=%u tid=%x code=%x\n",
 		(unsigned) current_event.dwProcessId,
 		(unsigned) current_event.dwThreadId,
-		current_event.dwDebugEventCode));
+		(unsigned) current_event.dwDebugEventCode));
       break;
     }
 

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch]: Replace stryoul call to fetch address

[Christopher Faylor]

On Wed, Feb 27, 2013 at 05:20:14PM +0000, Pedro Alves wrote:
>On 02/27/2013 04:44 PM, Corinna Vinschen wrote:
>> Hi,
>
>Hi,
>
>> 
>> here's a small patch which will help along building GDB on the (not
>> yet finished) x86_64 Cygwin.
>> 
>> Mostly these are just a few casts, and tweaks to the printf format
>> strings, so that the argument and the format matches all three cases:
>> 
>> - i686
>> - x86_64 LLP (Mingw, native Windows)
>> - x86_64 LP  (Cygwin)
>> 
>> The most important part of the patch is in handle_output_debug_string,
>> though.  The address for the context information is read from the string
>> using the strtoul function.  This works fine for a 32 bit GDB, and it
>> also works fine for a 64 bit Cygwin GDB.  It does not work for a 64 bit
>> Mingw GDB debugging a 64 bit Cygwin application, though.  Therefore I
>> replaced the strtoul with a call to string_to_core_addr.
>
>IIRC, the matching Cygwin code that that special
>Cygwin signals handling was never implemented, or it was disabled
>on Cygwin, or some such, and that gdb bits is actually causing
>trouble -- see http://sourceware.org/ml/gdb-patches/2013-02/msg00122.html.
>We should just zap it all.

I didn't take from that patch that it was causing problems.

The signal code is expected to work.

cgf

Re: [patch]: Replace stryoul call to fetch address

[Christopher Faylor]

On Wed, Feb 27, 2013 at 03:03:03PM -0500, Christopher Faylor wrote:
>On Wed, Feb 27, 2013 at 05:20:14PM +0000, Pedro Alves wrote:
>>On 02/27/2013 04:44 PM, Corinna Vinschen wrote:
>>> Hi,
>>
>>Hi,
>>
>>> 
>>> here's a small patch which will help along building GDB on the (not
>>> yet finished) x86_64 Cygwin.
>>> 
>>> Mostly these are just a few casts, and tweaks to the printf format
>>> strings, so that the argument and the format matches all three cases:
>>> 
>>> - i686
>>> - x86_64 LLP (Mingw, native Windows)
>>> - x86_64 LP  (Cygwin)
>>> 
>>> The most important part of the patch is in handle_output_debug_string,
>>> though.  The address for the context information is read from the string
>>> using the strtoul function.  This works fine for a 32 bit GDB, and it
>>> also works fine for a 64 bit Cygwin GDB.  It does not work for a 64 bit
>>> Mingw GDB debugging a 64 bit Cygwin application, though.  Therefore I
>>> replaced the strtoul with a call to string_to_core_addr.
>>
>>IIRC, the matching Cygwin code that that special
>>Cygwin signals handling was never implemented, or it was disabled
>>on Cygwin, or some such, and that gdb bits is actually causing
>>trouble -- see http://sourceware.org/ml/gdb-patches/2013-02/msg00122.html.
>>We should just zap it all.
>
>I didn't take from that patch that it was causing problems.
>
>The signal code is expected to work.

Just to be a little less terse: The quoted email is me commenting on the
fact that someone submitted a patch which "#if 0"ed some code.  I was
commenting on the '#if 0' not on the fact that cygwin's signal handling
with gdb is broken.  AFAIK, it is working.  If it isn't working then I
would never suggest that ripping out code was the right solution.

cgf

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Eli Zaretskii]

> Date: Wed, 27 Feb 2013 20:50:13 +0100
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> @@ -1452,7 +1452,7 @@ get_child_debug_event (struct target_wai
>      {
>      case CREATE_THREAD_DEBUG_EVENT:
>        OUTMSG2 (("gdbserver: kernel event CREATE_THREAD_DEBUG_EVENT "
> -		"for pid=%d tid=%x)\n",
> +		"for pid=%u tid=%x)\n",
>  		(unsigned) current_event.dwProcessId,
>  		(unsigned) current_event.dwThreadId));
>  
> @@ -1465,7 +1465,7 @@ get_child_debug_event (struct target_wai
>  
>      case EXIT_THREAD_DEBUG_EVENT:
>        OUTMSG2 (("gdbserver: kernel event EXIT_THREAD_DEBUG_EVENT "
> -		"for pid=%d tid=%x\n",
> +		"for pid=%u tid=%x\n",
>  		(unsigned) current_event.dwProcessId,
>  		(unsigned) current_event.dwThreadId));
>        child_delete_thread (current_event.dwProcessId,

I don't understand the need for any of these changes.  DWORD is
already an unsigned integer type:

  typedef unsigned long DWORD;

(this is from windef.h).  So why do we need to cast it to 'unsigned'??
What "type related warnings" did you see?

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Corinna Vinschen]

On Feb 27 22:23, Eli Zaretskii wrote:
> > Date: Wed, 27 Feb 2013 20:50:13 +0100
> > From: Corinna Vinschen <vinschen@redhat.com>
> > 
> > @@ -1452,7 +1452,7 @@ get_child_debug_event (struct target_wai
> >      {
> >      case CREATE_THREAD_DEBUG_EVENT:
> >        OUTMSG2 (("gdbserver: kernel event CREATE_THREAD_DEBUG_EVENT "
> > -		"for pid=%d tid=%x)\n",
> > +		"for pid=%u tid=%x)\n",
> >  		(unsigned) current_event.dwProcessId,
> >  		(unsigned) current_event.dwThreadId));
> >  
> > @@ -1465,7 +1465,7 @@ get_child_debug_event (struct target_wai
> >  
> >      case EXIT_THREAD_DEBUG_EVENT:
> >        OUTMSG2 (("gdbserver: kernel event EXIT_THREAD_DEBUG_EVENT "
> > -		"for pid=%d tid=%x\n",
> > +		"for pid=%u tid=%x\n",
> >  		(unsigned) current_event.dwProcessId,
> >  		(unsigned) current_event.dwThreadId));
> >        child_delete_thread (current_event.dwProcessId,
> 
> I don't understand the need for any of these changes.  DWORD is
> already an unsigned integer type:
> 
>   typedef unsigned long DWORD;
> 
> (this is from windef.h).  So why do we need to cast it to 'unsigned'??
> What "type related warnings" did you see?

If you look closely, I didn't add the unsigned casts.  I just changed
%d to %u or %x, matching the signedness of the value.  The unsigned
casts are already widely used throughout the file.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Eli Zaretskii]

> Date: Wed, 27 Feb 2013 21:29:02 +0100
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> If you look closely, I didn't add the unsigned casts.

Not in the part I cited (sorry), but elsewhere you did:

> @@ -1317,10 +1317,10 @@ handle_exception (struct target_waitstat
>  	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
>  	  return;
>  	}
> -      OUTMSG2 (("gdbserver: unknown target exception 0x%08lx at 0x%s",
> -		current_event.u.Exception.ExceptionRecord.ExceptionCode,
> -		phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
> -		ExceptionAddress, sizeof (uintptr_t))));
> +      OUTMSG2 (("gdbserver: unknown target exception 0x%08x at 0x%s",
> +	    (unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
> +	    phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
> +	    ExceptionAddress, sizeof (uintptr_t))));
[...]
> @@ -1561,10 +1561,10 @@ get_child_debug_event (struct target_wai
>  
>      default:
>        OUTMSG2 (("gdbserver: kernel event unknown "
> -		"for pid=%d tid=%x code=%ld\n",
> +		"for pid=%u tid=%x code=%x\n",
>  		(unsigned) current_event.dwProcessId,
>  		(unsigned) current_event.dwThreadId,
> -		current_event.dwDebugEventCode));
> +		(unsigned) current_event.dwDebugEventCode));
>        break;
>      }

Are those mistakes?

> The unsigned casts are already widely used throughout the file.

So why are they there?

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Pedro Alves]

On 02/27/2013 07:50 PM, Corinna Vinschen wrote:
> On Feb 27 20:42, Corinna Vinschen wrote:
>>>> Ok, I apply it then with only the int changed to unsigned.
>>>
>>> Thanks.
>>
>> Applied.
> 
> Here's an equivalent patch to gdbserver/win32-low.c.  Ok to apply?
> 

OK.

-- 
Pedro Alves

Re: [patch]: Replace stryoul call to fetch address

[Pedro Alves]

On 02/27/2013 08:05 PM, Christopher Faylor wrote:

> Just to be a little less terse: The quoted email is me commenting on the
> fact that someone submitted a patch which "#if 0"ed some code.  I was
> commenting on the '#if 0' not on the fact that cygwin's signal handling
> with gdb is broken.  AFAIK, it is working.  If it isn't working then I
> would never suggest that ripping out code was the right solution.

I was actually thinking of this:

 http://www.sourceware.org/ml/cygwin-developers/2007-02/msg00052.html

and IIRC some other messages on the cygwin lists in the past I
had found at the time of that question.  But it's useless to look
for them.

So you're saying that Cygwin actually does output the "cYgSiGw00f"
stuff nowadays?

-- 
Pedro Alves

Re: [patch]: Replace stryoul call to fetch address

[Pedro Alves]

On 02/27/2013 07:42 PM, Corinna Vinschen wrote:

> The SEGV occurs in exception.c, function throw_exception, though.
> The `*current_catcher->exception = exception;' assignment crashes
> because current_catcher->exception is NULL.  I don't understand yet
> why it's NULL, and why the throw_exception function doesn't test
> this before trying to write *current_catcher->exception.

What's the backtrace like?

There's always a top level catcher installed (gdb_main -> catch_errors)
Unless, hmm, waitaminut.  What's the backtrace like?  I just realized
a very early exception in captured_main can result in bad
things like that.

-- 
Pedro Alves

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Corinna Vinschen]

On Feb 27 23:05, Eli Zaretskii wrote:
> > Date: Wed, 27 Feb 2013 21:29:02 +0100
> > From: Corinna Vinschen <vinschen@redhat.com>
> > 
> > If you look closely, I didn't add the unsigned casts.
> 
> Not in the part I cited (sorry), but elsewhere you did:
> 
> > @@ -1317,10 +1317,10 @@ handle_exception (struct target_waitstat
> >  	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> >  	  return;
> >  	}
> > -      OUTMSG2 (("gdbserver: unknown target exception 0x%08lx at 0x%s",
> > -		current_event.u.Exception.ExceptionRecord.ExceptionCode,
> > -		phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
> > -		ExceptionAddress, sizeof (uintptr_t))));
> > +      OUTMSG2 (("gdbserver: unknown target exception 0x%08x at 0x%s",
> > +	    (unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
> > +	    phex_nz ((uintptr_t) current_event.u.Exception.ExceptionRecord.
> > +	    ExceptionAddress, sizeof (uintptr_t))));
> [...]
> > @@ -1561,10 +1561,10 @@ get_child_debug_event (struct target_wai
> >  
> >      default:
> >        OUTMSG2 (("gdbserver: kernel event unknown "
> > -		"for pid=%d tid=%x code=%ld\n",
> > +		"for pid=%u tid=%x code=%x\n",
> >  		(unsigned) current_event.dwProcessId,
> >  		(unsigned) current_event.dwThreadId,
> > -		current_event.dwDebugEventCode));
> > +		(unsigned) current_event.dwDebugEventCode));
> >        break;
> >      }
> 
> Are those mistakes?

No.

> > The unsigned casts are already widely used throughout the file.
> 
> So why are they there?

DWORD is unsigned long on 32 bit Windows, right?  So it is on 64 bit
Windows as well, since Windows is a LLP64 system.  64 bit Cygwin OTOH
will be LP64, so in 64 bit Cygwin's case, DWORD is defined as unsigned
int for hopefully obvious reasons.

So, using %ld with no cast will result in a GCC warning on 64 bit
Cygwin, because DWORD is not long.  %d would be right for 64 bit Cygwin,
but might result in a warning on 32 bit Mingw/Cygwin and 64 bit Mingw.

So, whatever they did before, the casts actually do serve a purpose.
They cast the variably defined DWORD-typed value to a well known int
type with a well defined printf format specifier.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch]: Replace stryoul call to fetch address

[Corinna Vinschen]

On Feb 27 21:50, Pedro Alves wrote:
> On 02/27/2013 07:42 PM, Corinna Vinschen wrote:
> 
> > The SEGV occurs in exception.c, function throw_exception, though.
> > The `*current_catcher->exception = exception;' assignment crashes
> > because current_catcher->exception is NULL.  I don't understand yet
> > why it's NULL, and why the throw_exception function doesn't test
> > this before trying to write *current_catcher->exception.
> 
> What's the backtrace like?
> 
> There's always a top level catcher installed (gdb_main -> catch_errors)
> Unless, hmm, waitaminut.  What's the backtrace like?  I just realized
> a very early exception in captured_main can result in bad
> things like that.

I didn't really debug this in depth yet.  Keep in mind that 64 bit
Cygwin is still in development so there are heinous bugs to be expected.
This crash is probably a result of an underlying Cygwin bug.

Nevertheless, here's a backtrace of the crashing `br dll_crt0_0'
command, as catched by Mingw GDB.  At this point in time, before starting
the executable, the dll_crt0_0 entry point is not known, since it's in
the not yet loaded Cygwin DLL.

#0  throw_exception (
    exception=<error reading variable: That operation is not available on integers of more than 8 bytes.>) at /home/corinna/src/gdb/src/gdb/exceptions.c:233
#1  0x00000001004f406e in throw_it (reason=RETURN_ERROR,
    error=<optimized out>, fmt=<optimized out>, ap=<optimized out>)
    at /home/corinna/src/gdb/src/gdb/exceptions.c:423
#2  0x00000001004f4287 in throw_error (error=4294096672,
    error@entry=NOT_FOUND_ERROR, fmt=0x40000 <Address 0x40000 out of bounds>)
    at /home/corinna/src/gdb/src/gdb/exceptions.c:444
#3  0x00000001004d56e5 in symbol_not_found_error (filename=0x0,
    symbol=0x6ffffeeb370 "\210_.\200\001")
    at /home/corinna/src/gdb/src/gdb/linespec.c:1410
#4  parse_linespec (parser=parser@entry=0xc2a110,
    argptr=argptr@entry=0xc2a488)
    at /home/corinna/src/gdb/src/gdb/linespec.c:2190
#5  0x00000001004d5972 in decode_line_full (argptr=0xc2a488,
    flags=<optimized out>, default_symtab=<optimized out>,
    default_line=<optimized out>, canonical=0xc2a410, select_mode=0x0,
    filter=0x0) at /home/corinna/src/gdb/src/gdb/linespec.c:2314
#6  0x0000000100489b42 in parse_breakpoint_sals (address=0xc2a488,
    canonical=0xc2a410) at /home/corinna/src/gdb/src/gdb/breakpoint.c:9304
#7  0x000000010048f32e in create_breakpoint (gdbarch=0x6fffff3bd20,
    arg=0x6fffffebb9d "", cond_string=0x0, thread=0, extra_string=0x0,
    parse_condition_and_thread=1, tempflag=0, type_wanted=bp_breakpoint,
    ignore_count=0, pending_break_support=AUTO_BOOLEAN_AUTO,
    ops=0x10085fd40 <bkpt_breakpoint_ops>, from_tty=1, enabled=1, internal=0,
    flags=0) at /home/corinna/src/gdb/src/gdb/breakpoint.c:9535
#8  0x000000010048fbd9 in break_command_1 (arg=0x6fffffebb93 "dll_crt0_0",
    flag=<optimized out>, from_tty=1)
    at /home/corinna/src/gdb/src/gdb/breakpoint.c:9753
#9  0x00000001005ae77b in execute_command (p=0x6fffffebb9c "0",
    p@entry=0x6fffffebb90 "br dll_crt0_0", from_tty=1)
    at /home/corinna/src/gdb/src/gdb/top.c:484
#10 0x00000001004fcd47 in command_handler (
    command=0x6fffffebb90 "br dll_crt0_0")
    at /home/corinna/src/gdb/src/gdb/event-top.c:431
#11 0x00000001004fd0fc in command_line_handler (rl=<optimized out>)
    at /home/corinna/src/gdb/src/gdb/event-top.c:629
#12 0x00000001005e1120 in rl_callback_read_char ()
    at /home/corinna/src/gdb/src/readline/callback.c:220
#13 0x00000001004fcdb9 in rl_callback_read_char_wrapper (
    client_data=<optimized out>)
    at /home/corinna/src/gdb/src/gdb/event-top.c:163
#14 0x00000001004fb954 in process_event ()
    at /home/corinna/src/gdb/src/gdb/event-loop.c:342
#15 0x00000001004fbcd7 in gdb_do_one_event ()
    at /home/corinna/src/gdb/src/gdb/event-loop.c:406
#16 0x00000001004fbf2e in start_event_loop ()
    at /home/corinna/src/gdb/src/gdb/event-loop.c:431
#17 0x00000001004f5863 in captured_command_loop (data=data@entry=0x0)
    at /home/corinna/src/gdb/src/gdb/main.c:256
#18 0x00000001004f451a in catch_errors (
    func=func@entry=0x1004f5850 <captured_command_loop>,
    func_args=func_args@entry=0x0,
    errstring=errstring@entry=0x10070c771 <__PRETTY_FUNCTION__.12619+219> "",
    mask=mask@entry=6) at /home/corinna/src/gdb/src/gdb/exceptions.c:546
#19 0x00000001004f65bb in captured_main (data=data@entry=0xc2aaa0)
    at /home/corinna/src/gdb/src/gdb/main.c:1033
#20 0x00000001004f451a in catch_errors (
    func=func@entry=0x1004f5b40 <captured_main>,
    func_args=func_args@entry=0xc2aaa0,
    errstring=errstring@entry=0x10070c771 <__PRETTY_FUNCTION__.12619+219> "",
    mask=mask@entry=6) at /home/corinna/src/gdb/src/gdb/exceptions.c:546
#21 0x00000001004f6b79 in gdb_main (args=args@entry=0xc2aaa0)
    at /home/corinna/src/gdb/src/gdb/main.c:1042
#22 0x00000001006a7131 in main (argc=2, argv=0xc2ab00)
    at /home/corinna/src/gdb/src/gdb/gdb.c:34


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Corinna Vinschen]

On Feb 27 21:25, Pedro Alves wrote:
> On 02/27/2013 07:50 PM, Corinna Vinschen wrote:
> > On Feb 27 20:42, Corinna Vinschen wrote:
> >>>> Ok, I apply it then with only the int changed to unsigned.
> >>>
> >>> Thanks.
> >>
> >> Applied.
> > 
> > Here's an equivalent patch to gdbserver/win32-low.c.  Ok to apply?
> > 
> 
> OK.

Thanks, applied.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch] gdbserver/win32-low.c: Fix printf-like formatting (was Re: [patch]: Replace stryoul call to fetch address)

[Eli Zaretskii]

> Date: Thu, 28 Feb 2013 09:54:26 +0100
> From: Corinna Vinschen <vinschen@redhat.com>
> 
> DWORD is unsigned long on 32 bit Windows, right?  So it is on 64 bit
> Windows as well, since Windows is a LLP64 system.  64 bit Cygwin OTOH
> will be LP64, so in 64 bit Cygwin's case, DWORD is defined as unsigned
> int for hopefully obvious reasons.
> 
> So, using %ld with no cast will result in a GCC warning on 64 bit
> Cygwin, because DWORD is not long.  %d would be right for 64 bit Cygwin,
> but might result in a warning on 32 bit Mingw/Cygwin and 64 bit Mingw.

Thanks.

(I think it's madness to use LP64 for Cygwin, but I doubt anyone will
hear me.)

Re: [patch]: Replace stryoul call to fetch address

[Corinna Vinschen]

On Feb 28 10:18, Corinna Vinschen wrote:
> On Feb 27 21:50, Pedro Alves wrote:
> > On 02/27/2013 07:42 PM, Corinna Vinschen wrote:
> > 
> > > The SEGV occurs in exception.c, function throw_exception, though.
> > > The `*current_catcher->exception = exception;' assignment crashes
> > > because current_catcher->exception is NULL.  I don't understand yet
> > > why it's NULL, and why the throw_exception function doesn't test
> > > this before trying to write *current_catcher->exception.
> > 
> > What's the backtrace like?
> > 
> > There's always a top level catcher installed (gdb_main -> catch_errors)
> > Unless, hmm, waitaminut.  What's the backtrace like?  I just realized
> > a very early exception in captured_main can result in bad
> > things like that.
> 
> I didn't really debug this in depth yet.  Keep in mind that 64 bit
> Cygwin is still in development so there are heinous bugs to be expected.
> This crash is probably a result of an underlying Cygwin bug.

I debugged this further and it seems this is a bug in newlib's
definition of setjmp_buf:

typedef _JBTYPE sigjmp_buf[_JBLEN+1+(sizeof (sigset_t)/sizeof (_JBTYPE))];

If sizeof(sigset_t) is less than sizeof(_JBTYPE), then the result of the
division is zero, and the buffer is too short by sizeof(sigset_t).
The element preceeding the exception pointer in `struct catcher' is a
sigjmp_buf.  So exception is NULL, because the sigsetjmp call overwrites
exeception with a signal mask.

I'm going to replace the expression in newlib's setjmp.h with

typedef _JBTYPE sigjmp_buf[_JBLEN+1+((sizeof (_JBTYPE) + sizeof (sigset_t) - 1)
                                     /sizeof (_JBTYPE))];

which makes sure that the result of the division is at least 1.  This
change requires to rebuild the toolchain from scratch so it will take
some time to see the result of the change.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch]: Replace stryoul call to fetch address

[Corinna Vinschen]

On Feb 28 17:20, Corinna Vinschen wrote:
> On Feb 28 10:18, Corinna Vinschen wrote:
> > On Feb 27 21:50, Pedro Alves wrote:
> > > On 02/27/2013 07:42 PM, Corinna Vinschen wrote:
> > > 
> > > > The SEGV occurs in exception.c, function throw_exception, though.
> > > > The `*current_catcher->exception = exception;' assignment crashes
> > > > because current_catcher->exception is NULL.  I don't understand yet
> > > > why it's NULL, and why the throw_exception function doesn't test
> > > > this before trying to write *current_catcher->exception.
> > > 
> > > What's the backtrace like?
> > > 
> > > There's always a top level catcher installed (gdb_main -> catch_errors)
> > > Unless, hmm, waitaminut.  What's the backtrace like?  I just realized
> > > a very early exception in captured_main can result in bad
> > > things like that.
> > 
> > I didn't really debug this in depth yet.  Keep in mind that 64 bit
> > Cygwin is still in development so there are heinous bugs to be expected.
> > This crash is probably a result of an underlying Cygwin bug.
> 
> I debugged this further and it seems this is a bug in newlib's
> definition of setjmp_buf:
> 
> typedef _JBTYPE sigjmp_buf[_JBLEN+1+(sizeof (sigset_t)/sizeof (_JBTYPE))];
> 
> If sizeof(sigset_t) is less than sizeof(_JBTYPE), then the result of the
> division is zero, and the buffer is too short by sizeof(sigset_t).
> The element preceeding the exception pointer in `struct catcher' is a
> sigjmp_buf.  So exception is NULL, because the sigsetjmp call overwrites
> exeception with a signal mask.
> 
> I'm going to replace the expression in newlib's setjmp.h with
> 
> typedef _JBTYPE sigjmp_buf[_JBLEN+1+((sizeof (_JBTYPE) + sizeof (sigset_t) - 1)
>                                      /sizeof (_JBTYPE))];
> 
> which makes sure that the result of the division is at least 1.  This
> change requires to rebuild the toolchain from scratch so it will take
> some time to see the result of the change.

Well, not *that* long, actually.  This patch did it.  x86_64 GDB is up
and running :)


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Re: [patch]: Replace stryoul call to fetch address

[Pedro Alves]

On 02/28/2013 04:29 PM, Corinna Vinschen wrote:
> Well, not *that* long, actually.  This patch did it.  x86_64 GDB is up
> and running :)

Yay! :-)

-- 
Pedro Alves