[RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Pierre Muller]

This patch fixes the build failure for cygwin (or mingw32)
if you add --enable-64-bit-bfd option to configure.

  When I tried to check the multi-target support
patch from Ulrich Weigand, I stumpled over a
win32 native problem.
See
  http://sourceware.org/ml/gdb-patches/2007-11/msg00367.html

The problem is that win32-nat.c code implicitly supposes
the CORE_ADDR is a 32 bit type, while it becomes a
64 bit type if --enable-64-bit-bfd option is used.


  The testsuite gives very similar results
for gdb configured without options and
with --enable-64-bit-bfd after that patch.

Pierre Muller

The ChangeLog entry is maybe to verbose, but
I didn't know how to shorten it.


ChangeLog entry:

2007-11-22  Pierre Muller  <muller@ics.u-strasbg.fr>

	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
	Add "gdb_stdint.h" dependency required for uintptr_t type use.
	(handle_output_debug_string): New local variable: addr,
	used to convert lpDebugStringData pointer to uintptr_t type, which
can further be
	typecasted to CORE_ADDR.
	(handle_exception): Typecast ExceptionAddress field to uintptr_t.
	(win32_xfer_memory): New local variable: mem_addr,
	used to convert memaddr of type CORE_ADDR to uintputr_t.



Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.139
diff -u -p -r1.139 win32-nat.c
--- gdb/win32-nat.c	16 Nov 2007 04:53:46 -0000	1.139
+++ gdb/win32-nat.c	23 Nov 2007 08:30:33 -0000
@@ -48,6 +48,7 @@
 #include "objfiles.h"
 #include "gdb_obstack.h"
 #include "gdb_string.h"
+#include "gdb_stdint.h"
 #include "gdbthread.h"
 #include "gdbcmd.h"
 #include <sys/param.h>
@@ -818,9 +819,10 @@ handle_output_debug_string (struct targe
 {
   char *s = NULL;
   int retval = 0;
+  uintptr_t addr = (uintptr_t)
current_event.u.DebugString.lpDebugStringData;
 
   if (!target_read_string
-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
0)
+    ((CORE_ADDR) addr, &s, 1024, 0)
       || !s || !*s)
     /* nothing to do */;
   else if (strncmp (s, _CYGWIN_SIGNAL_STRING, sizeof
(_CYGWIN_SIGNAL_STRING) - 1) != 0)
@@ -1014,7 +1016,7 @@ handle_exception (struct target_waitstat
 	   and will be sent as a cygwin-specific-signal.  So, ignore SEGVs
if they show up
 	   within the text segment of the DLL itself. */
 	char *fn;
-	bfd_vma addr = (bfd_vma)
current_event.u.Exception.ExceptionRecord.ExceptionAddress;
+	bfd_vma addr = (uintptr_t)
current_event.u.Exception.ExceptionRecord.ExceptionAddress;
 	if ((!cygwin_exceptions && (addr >= cygwin_load_start && addr <
cygwin_load_end))
 	    || (find_pc_partial_function (addr, &fn, NULL, NULL)
 		&& strncmp (fn, "KERNEL32!IsBad", strlen ("KERNEL32!IsBad"))
== 0))
@@ -1915,20 +1917,22 @@ win32_xfer_memory (CORE_ADDR memaddr, gd
 		   struct target_ops *target)
 {
   DWORD done = 0;
+  uintptr_t mem_addr = (uintptr_t) memaddr;
+
   if (write)
     {
       DEBUG_MEM (("gdb: write target memory, %d bytes at 0x%08lx\n",
-		  len, (DWORD) memaddr));
-      if (!WriteProcessMemory (current_process_handle, (LPVOID) memaddr,
our,
+		  len, (DWORD) mem_addr));
+      if (!WriteProcessMemory (current_process_handle, (LPVOID) mem_addr,
our,
 			       len, &done))
 	done = 0;
-      FlushInstructionCache (current_process_handle, (LPCVOID) memaddr,
len);
+      FlushInstructionCache (current_process_handle, (LPCVOID) mem_addr,
len);
     }
   else
     {
       DEBUG_MEM (("gdb: read target memory, %d bytes at 0x%08lx\n",
-		  len, (DWORD) memaddr));
-      if (!ReadProcessMemory (current_process_handle, (LPCVOID) memaddr,
our,
+		  len, (DWORD) mem_addr));
+      if (!ReadProcessMemory (current_process_handle, (LPCVOID) mem_addr,
our,
 			      len, &done))
 	done = 0;
     }

Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Christopher Faylor]

On Fri, Nov 23, 2007 at 09:50:27AM +0100, Pierre Muller wrote:
>This patch fixes the build failure for cygwin (or mingw32)
>if you add --enable-64-bit-bfd option to configure.
>
>  When I tried to check the multi-target support
>patch from Ulrich Weigand, I stumpled over a
>win32 native problem.
>See
>  http://sourceware.org/ml/gdb-patches/2007-11/msg00367.html
>
>The problem is that win32-nat.c code implicitly supposes
>the CORE_ADDR is a 32 bit type, while it becomes a
>64 bit type if --enable-64-bit-bfd option is used.
>
>
>  The testsuite gives very similar results
>for gdb configured without options and
>with --enable-64-bit-bfd after that patch.
>
>Pierre Muller
>
>The ChangeLog entry is maybe to verbose, but
>I didn't know how to shorten it.
>
>
>ChangeLog entry:
>
>2007-11-22  Pierre Muller  <muller@ics.u-strasbg.fr>
>
>	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
>	Add "gdb_stdint.h" dependency required for uintptr_t type use.
>	(handle_output_debug_string): New local variable: addr,
>	used to convert lpDebugStringData pointer to uintptr_t type, which
>can further be
>	typecasted to CORE_ADDR.
>	(handle_exception): Typecast ExceptionAddress field to uintptr_t.
>	(win32_xfer_memory): New local variable: mem_addr,
>	used to convert memaddr of type CORE_ADDR to uintputr_t.
>
>
>
>Index: gdb/win32-nat.c
>===================================================================
>RCS file: /cvs/src/src/gdb/win32-nat.c,v
>retrieving revision 1.139
>diff -u -p -r1.139 win32-nat.c
>--- gdb/win32-nat.c	16 Nov 2007 04:53:46 -0000	1.139
>+++ gdb/win32-nat.c	23 Nov 2007 08:30:33 -0000
>@@ -48,6 +48,7 @@
> #include "objfiles.h"
> #include "gdb_obstack.h"
> #include "gdb_string.h"
>+#include "gdb_stdint.h"
> #include "gdbthread.h"
> #include "gdbcmd.h"
> #include <sys/param.h>
>@@ -818,9 +819,10 @@ handle_output_debug_string (struct targe
> {
>   char *s = NULL;
>   int retval = 0;
>+  uintptr_t addr = (uintptr_t)
>current_event.u.DebugString.lpDebugStringData;
> 
>   if (!target_read_string
>-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
>0)
>+    ((CORE_ADDR) addr, &s, 1024, 0)

How can coercing something to uintptr_t and then to CORE_ADDR achieve
anything?  How does the double coercion help?

>       || !s || !*s)
>     /* nothing to do */;
>   else if (strncmp (s, _CYGWIN_SIGNAL_STRING, sizeof
>(_CYGWIN_SIGNAL_STRING) - 1) != 0)
>@@ -1014,7 +1016,7 @@ handle_exception (struct target_waitstat
> 	   and will be sent as a cygwin-specific-signal.  So, ignore SEGVs
>if they show up
> 	   within the text segment of the DLL itself. */
> 	char *fn;
>-	bfd_vma addr = (bfd_vma)
>current_event.u.Exception.ExceptionRecord.ExceptionAddress;
>+	bfd_vma addr = (uintptr_t)
>current_event.u.Exception.ExceptionRecord.ExceptionAddress;

Same question here.  Since you are assigning to a bfd_vma how does the
above help?  Does it avoid a warning or something?

> 	if ((!cygwin_exceptions && (addr >= cygwin_load_start && addr <
>cygwin_load_end))
> 	    || (find_pc_partial_function (addr, &fn, NULL, NULL)
> 		&& strncmp (fn, "KERNEL32!IsBad", strlen ("KERNEL32!IsBad"))
>== 0))
>@@ -1915,20 +1917,22 @@ win32_xfer_memory (CORE_ADDR memaddr, gd
> 		   struct target_ops *target)
> {
>   DWORD done = 0;
>+  uintptr_t mem_addr = (uintptr_t) memaddr;
>+
>   if (write)
>     {
>       DEBUG_MEM (("gdb: write target memory, %d bytes at 0x%08lx\n",
>-		  len, (DWORD) memaddr));
>-      if (!WriteProcessMemory (current_process_handle, (LPVOID) memaddr,
>our,
>+		  len, (DWORD) mem_addr));
>+      if (!WriteProcessMemory (current_process_handle, (LPVOID) mem_addr,
>our,
> 			       len, &done))

Ugh.  Having two variables named "mem_addr" and "memaddr" is a recipe
for confusion.  But, it's the same question: since you're performing a
coercion, why does setting something to a variable earlier in the code
help?

etc.

cgf

Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Daniel Jacobowitz]

On Sat, Nov 24, 2007 at 04:07:08PM -0500, Christopher Faylor wrote:
> >   if (!target_read_string
> >-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
> >0)
> >+    ((CORE_ADDR) addr, &s, 1024, 0)
> 
> How can coercing something to uintptr_t and then to CORE_ADDR achieve
> anything?  How does the double coercion help?

Just the warning.  CORE_ADDR will be long long,
current_event.u.DebugString.lpDebugStringData will apparently be a pointer.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Christopher Faylor]

On Sat, Nov 24, 2007 at 05:47:27PM -0500, Daniel Jacobowitz wrote:
>On Sat, Nov 24, 2007 at 04:07:08PM -0500, Christopher Faylor wrote:
>> >   if (!target_read_string
>> >-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
>> >0)
>> >+    ((CORE_ADDR) addr, &s, 1024, 0)
>> 
>> How can coercing something to uintptr_t and then to CORE_ADDR achieve
>> anything?  How does the double coercion help?
>
>Just the warning.  CORE_ADDR will be long long,
>current_event.u.DebugString.lpDebugStringData will apparently be a pointer.

And the warning is?

If this is a problem then it seems like defining a macro (macros are
good m'kay?) to do the coercion rather than sprinkling temporary
variables throughout the code might be a better way to go.

cgf

Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Daniel Jacobowitz]

On Sun, Nov 25, 2007 at 12:32:08PM -0500, Christopher Faylor wrote:
> On Sat, Nov 24, 2007 at 05:47:27PM -0500, Daniel Jacobowitz wrote:
> >On Sat, Nov 24, 2007 at 04:07:08PM -0500, Christopher Faylor wrote:
> >> >   if (!target_read_string
> >> >-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
> >> >0)
> >> >+    ((CORE_ADDR) addr, &s, 1024, 0)
> >> 
> >> How can coercing something to uintptr_t and then to CORE_ADDR achieve
> >> anything?  How does the double coercion help?
> >
> >Just the warning.  CORE_ADDR will be long long,
> >current_event.u.DebugString.lpDebugStringData will apparently be a pointer.
> 
> And the warning is?

Cast from pointer to integer of different size.  Casts are the way
we've handled it elsewhere in GDB, but I wouldn't complain about a
wrapper; casting host pointers to CORE_ADDRs is an action we try to
keep to a minimum anyway.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Christopher Faylor]

On Sun, Nov 25, 2007 at 02:08:23PM -0500, Daniel Jacobowitz wrote:
>On Sun, Nov 25, 2007 at 12:32:08PM -0500, Christopher Faylor wrote:
>> On Sat, Nov 24, 2007 at 05:47:27PM -0500, Daniel Jacobowitz wrote:
>> >On Sat, Nov 24, 2007 at 04:07:08PM -0500, Christopher Faylor wrote:
>> >> >   if (!target_read_string
>> >> >-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
>> >> >0)
>> >> >+    ((CORE_ADDR) addr, &s, 1024, 0)
>> >> 
>> >> How can coercing something to uintptr_t and then to CORE_ADDR achieve
>> >> anything?  How does the double coercion help?
>> >
>> >Just the warning.  CORE_ADDR will be long long,
>> >current_event.u.DebugString.lpDebugStringData will apparently be a pointer.
>> 
>> And the warning is?
>
>Cast from pointer to integer of different size.  Casts are the way
>we've handled it elsewhere in GDB, but I wouldn't complain about a
>wrapper; casting host pointers to CORE_ADDRs is an action we try to
>keep to a minimum anyway.

I wouldn't mind a double cast either, if there is precedent for that.

cgf

RE: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Pierre Muller]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Christopher Faylor
> Sent: Sunday, November 25, 2007 11:13 PM
> To: gdb-patches@sourceware.org
> Subject: Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-
> bit-bfd
> 
> On Sun, Nov 25, 2007 at 02:08:23PM -0500, Daniel Jacobowitz wrote:
> >On Sun, Nov 25, 2007 at 12:32:08PM -0500, Christopher Faylor wrote:
> >> On Sat, Nov 24, 2007 at 05:47:27PM -0500, Daniel Jacobowitz wrote:
> >> >On Sat, Nov 24, 2007 at 04:07:08PM -0500, Christopher Faylor wrote:
> >> >> >   if (!target_read_string
> >> >> >-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData,
> &s, 1024,
> >> >> >0)
> >> >> >+    ((CORE_ADDR) addr, &s, 1024, 0)
> >> >>
> >> >> How can coercing something to uintptr_t and then to CORE_ADDR
> achieve
> >> >> anything?  How does the double coercion help?
> >> >
> >> >Just the warning.  CORE_ADDR will be long long,
> >> >current_event.u.DebugString.lpDebugStringData will apparently be a
> pointer.
> >>
> >> And the warning is?
> >
> >Cast from pointer to integer of different size.  Casts are the way
> >we've handled it elsewhere in GDB, but I wouldn't complain about a
> >wrapper; casting host pointers to CORE_ADDRs is an action we try to
> >keep to a minimum anyway.
> 
> I wouldn't mind a double cast either, if there is precedent for that.

  I didn't find many double typecast in gdb directory:
grep "([a-zA-Z0-9 ]*) *([a-zA-Z0-9 ]*)" *nat* 
only found one occurrence:
spu-linux-nat.c:  return (ULONGEST) (unsigned long) res;
and this does not even seem to be a pointer<->integer cast...

  Please remember that my C knowledge is mainly limited to
the gdb sources themselves...
  so if I have a CORE_ADDR that could be 64 bit
and I want to cast it to a pointer, I should use
  (LPVOID) (uintptr_t) core_addr
and if I have a win32 API pointer that I want to
convert to a CORE_ADDR, I should use
  (CORE_ADDR) (uintptr_t) pointer_var.

  My patch contained two conversions of the first type and
four of the second type (all with the same memaddr variable)
are macros really useful? Maybe to avoid future
similar problems...

  If we go for macros, I have no idea how to name those.
  
#define CORE_ADDR_TO_POINTER (LPVOID) (uintptr_t)
and
#define POINTER_TO_CORE_ADDR (CORE_ADDR) (uintptr_t)

I don't know if more braces are needed or not...

  It's probably best to still use uintptr_t as 
Pedro was talking about trying to support win64, no?

Pierre

Re: [RFC/RFA] Allow cygwin native to compile with --enable-64-bit-bfd

[Ulrich Weigand]

Pierre Muller wrote:

>   I didn't find many double typecast in gdb directory:
> grep "([a-zA-Z0-9 ]*) *([a-zA-Z0-9 ]*)" *nat* 
> only found one occurrence:
> spu-linux-nat.c:  return (ULONGEST) (unsigned long) res;
> and this does not even seem to be a pointer<->integer cast...

Your regexp is missing "_", so it actually cannot match the
cases involving intptr_t.  It's true that even so there is
not a large number of precedents, but that is because the
operation of casting between host pointers and CORE_ADDR
is quite rare (and that is how it should be!):

inf-ptrace.c:                               (PTRACE_TYPE_ARG3)(uintptr_t)
inf-ptrace.c:               (PTRACE_TYPE_ARG3)(uintptr_t)rounded_offset,
inf-ptrace.c:                   (PTRACE_TYPE_ARG3)(uintptr_t)rounded_offset,
inf-ptrace.c:                             (PTRACE_TYPE_ARG3)(uintptr_t)rounded_offset,
inf-ptrace.c:      buf[i] = ptrace (PT_READ_U, pid, (PTRACE_TYPE_ARG3)(uintptr_t)addr, 0);
inf-ptrace.c:      ptrace (PT_WRITE_U, pid, (PTRACE_TYPE_ARG3)(uintptr_t)addr, buf[i]);
linux-thread-db.c:           ? (CORE_ADDR) (intptr_t) notify.u.bptaddr
linux-thread-db.c:           : (CORE_ADDR) (uintptr_t) notify.u.bptaddr),
linux-thread-db.c:            ? (CORE_ADDR) (intptr_t) address
linux-thread-db.c:            : (CORE_ADDR) (uintptr_t) address);
ppc-linux-nat.c:  *addr_p = (CORE_ADDR) (uintptr_t) siginfo_p->si_addr;
proc-service.c:    return (psaddr_t) (intptr_t) addr;
proc-service.c:    return (psaddr_t) (uintptr_t) addr;
spu-linux-nat.c:  return (ULONGEST) (unsigned long) res;
spu-linux-nat.c:    *word = ptrace (PT_READ_I, tid, (PTRACE_TYPE_ARG3) (size_t) memaddr, 0);
spu-linux-nat.c:    ptrace (PT_WRITE_D, tid, (PTRACE_TYPE_ARG3) (size_t) memaddr, word);

>   Please remember that my C knowledge is mainly limited to
> the gdb sources themselves...
>   so if I have a CORE_ADDR that could be 64 bit
> and I want to cast it to a pointer, I should use
>   (LPVOID) (uintptr_t) core_addr
> and if I have a win32 API pointer that I want to
> convert to a CORE_ADDR, I should use
>   (CORE_ADDR) (uintptr_t) pointer_var.

Basically, yes.  Except that "LPVOID" is rather a Windows-ism,
it is not actually defined anywhere else ...

You should be using "void *", or preferably the actual
target pointer type.

>   My patch contained two conversions of the first type and
> four of the second type (all with the same memaddr variable)
> are macros really useful? Maybe to avoid future
> similar problems...

I'd prefer to not have macros, but just an explicitly
written intermediate cast.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

[RFA v2] Allow cygwin native to compile with --enable-64-bit-bfd

[Pierre Muller]

> >> And the warning is?
> >
> >Cast from pointer to integer of different size.  Casts are the way
> >we've handled it elsewhere in GDB, but I wouldn't complain about a
> >wrapper; casting host pointers to CORE_ADDRs is an action we try to
> >keep to a minimum anyway.
> 
> I wouldn't mind a double cast either, if there is precedent for that.

 Here is a revised patch that only uses double typecasts.
 I ran the testsuite and got a slight improvement (2 FAIL less in
gdb.base/signals.exp),
but I doubt this is significant...

OK to check in?

ChangeLog entry:

2007-11-28  Pierre Muller  <muller@ics.u-strasbg.fr>

	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
	Add "gdb_stdint.h" dependency required for uintptr_t type use.
	(handle_output_debug_string): Use uintptr_t typecast.
	(handle_exception): Ditto.
	(win32_xfer_memory): Ditto.

Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.140
diff -u -p -r1.140 win32-nat.c
--- gdb/win32-nat.c     24 Nov 2007 12:13:28 -0000      1.140
+++ gdb/win32-nat.c     28 Nov 2007 11:02:09 -0000
@@ -48,6 +48,7 @@
 #include "objfiles.h"
 #include "gdb_obstack.h"
 #include "gdb_string.h"
+#include "gdb_stdint.h"
 #include "gdbthread.h"
 #include "gdbcmd.h"
 #include <sys/param.h>
@@ -829,7 +830,8 @@ handle_output_debug_string (struct targe
   int retval = 0;

   if (!target_read_string
-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
0)
+       ((CORE_ADDR) (uintptr_t)
current_event.u.DebugString.lpDebugStringData,
+       &s, 1024, 0)
       || !s || !*s)
     /* nothing to do */;
   else if (strncmp (s, _CYGWIN_SIGNAL_STRING, sizeof
(_CYGWIN_SIGNAL_STRING) -
1) != 0)
@@ -1023,7 +1025,8 @@ handle_exception (struct target_waitstat
           and will be sent as a cygwin-specific-signal.  So, ignore SEGVs
if th
ey show up
           within the text segment of the DLL itself. */
        char *fn;
-       bfd_vma addr = (bfd_vma)
current_event.u.Exception.ExceptionRecord.Excep
tionAddress;
+       bfd_vma addr = (bfd_vma) (uintptr_t) current_event.u.Exception.
+
ExceptionRecord.ExceptionAddress;
        if ((!cygwin_exceptions && (addr >= cygwin_load_start && addr <
cygwin_l
oad_end))
            || (find_pc_partial_function (addr, &fn, NULL, NULL)
                && strncmp (fn, "KERNEL32!IsBad", strlen ("KERNEL32!IsBad"))
==
0))
@@ -1925,20 +1928,24 @@ win32_xfer_memory (CORE_ADDR memaddr, gd
                   struct target_ops *target)
 {
   DWORD done = 0;
+
   if (write)
     {
       DEBUG_MEM (("gdb: write target memory, %d bytes at 0x%08lx\n",
-                 len, (DWORD) memaddr));
-      if (!WriteProcessMemory (current_process_handle, (LPVOID) memaddr,
our,
+                 len, (DWORD) (uintptr_t) memaddr));
+      if (!WriteProcessMemory (current_process_handle,
+                              (LPVOID) (uintptr_t) memaddr, our,
                               len, &done))
        done = 0;
-      FlushInstructionCache (current_process_handle, (LPCVOID) memaddr,
len);
+      FlushInstructionCache (current_process_handle,
+                            (LPCVOID) (uintptr_t) memaddr, len);
     }
   else
     {
       DEBUG_MEM (("gdb: read target memory, %d bytes at 0x%08lx\n",
-                 len, (DWORD) memaddr));
-      if (!ReadProcessMemory (current_process_handle, (LPCVOID) memaddr,
our,
+                 len, (DWORD) (uintptr_t) memaddr));
+      if (!ReadProcessMemory (current_process_handle,
+                             (LPCVOID) (uintptr_t) memaddr, our,
                              len, &done))
        done = 0;
     }

Re: [RFA v2] Allow cygwin native to compile with --enable-64-bit-bfd

[Christopher Faylor]

On Thu, Nov 29, 2007 at 11:11:11AM +0100, Pierre Muller wrote:
>> >> And the warning is?
>> >
>> >Cast from pointer to integer of different size.  Casts are the way
>> >we've handled it elsewhere in GDB, but I wouldn't complain about a
>> >wrapper; casting host pointers to CORE_ADDRs is an action we try to
>> >keep to a minimum anyway.
>> 
>> I wouldn't mind a double cast either, if there is precedent for that.
>
> Here is a revised patch that only uses double typecasts.
> I ran the testsuite and got a slight improvement (2 FAIL less in
>gdb.base/signals.exp),
>but I doubt this is significant...
>
>OK to check in?

I'd like to get opinions from other maintainers on the use of a macro
for this case.  I don't like seeing unexplained double casts like this
and I think a macro could make it clearer.

Daniel were you implying that you would just tolerate a macro here or
do you think it's an ok idea.

cgf

>ChangeLog entry:
>
>2007-11-28  Pierre Muller  <muller@ics.u-strasbg.fr>
>
>	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
>	Add "gdb_stdint.h" dependency required for uintptr_t type use.
>	(handle_output_debug_string): Use uintptr_t typecast.
>	(handle_exception): Ditto.
>	(win32_xfer_memory): Ditto.

Re: [RFA v2] Allow cygwin native to compile with --enable-64-bit-bfd

[Daniel Jacobowitz]

On Sat, Dec 01, 2007 at 09:43:11PM -0500, Christopher Faylor wrote:
> I'd like to get opinions from other maintainers on the use of a macro
> for this case.  I don't like seeing unexplained double casts like this
> and I think a macro could make it clearer.
> 
> Daniel were you implying that you would just tolerate a macro here or
> do you think it's an ok idea.

I'm very familiar with this idiom, so the double cast stands out when
I see it.  If there were a macro I'd have to go figure out whether it
was doing something more subtle.  I'd write it this way, but not
complain if someone else wrote it differently.

> >2007-11-28  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> >	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
> >	Add "gdb_stdint.h" dependency required for uintptr_t type use.

Added includes -> update Makefile.in.  Tom's been working on automatic
dependencies for GCC.  Maybe we can do something similar for GDB
soon...

-- 
Daniel Jacobowitz
CodeSourcery

[RFA v3] Allow cygwin native to compile with --enable-64-bit-bfd

[Pierre Muller]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Daniel Jacobowitz
> Sent: Sunday, December 02, 2007 5:00 AM
> To: gdb-patches@sourceware.org
> Cc: Pierre Muller
> Subject: Re: [RFA v2] Allow cygwin native to compile with --enable-64-
> bit-bfd
> 
> On Sat, Dec 01, 2007 at 09:43:11PM -0500, Christopher Faylor wrote:
> > I'd like to get opinions from other maintainers on the use of a macro
> > for this case.  I don't like seeing unexplained double casts like
> this
> > and I think a macro could make it clearer.
> >
> > Daniel were you implying that you would just tolerate a macro here or
> > do you think it's an ok idea.
> 
> I'm very familiar with this idiom, so the double cast stands out when
> I see it.  If there were a macro I'd have to go figure out whether it
> was doing something more subtle.  I'd write it this way, but not
> complain if someone else wrote it differently.
> 
> > >2007-11-28  Pierre Muller  <muller@ics.u-strasbg.fr>
> > >
> > >	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
> > >	Add "gdb_stdint.h" dependency required for uintptr_t type use.
> 
> Added includes -> update Makefile.in.  Tom's been working on automatic
> dependencies for GCC.  Maybe we can do something similar for GDB
> soon...

 I modified the patch to add the change to Makefile.in.


  After checking again that there was
no change to the testsuite. 

OK to commit?


2007-12-03  Pierre Muller  <muller@ics.u-strasbg.fr>

	* win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
	Add "gdb_stdint.h" dependency required for uintptr_t type use.
	(handle_output_debug_string): Use uintptr_t typecast.
	(handle_exception): Ditto.
	(win32_xfer_memory): Ditto.

	* Makefile.in (win32-nat.o): Add dependency to gdb_stdint header.


Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.963
diff -u -p -r1.963 Makefile.in
--- gdb/Makefile.in	30 Nov 2007 21:50:18 -0000	1.963
+++ gdb/Makefile.in	2 Dec 2007 21:43:10 -0000
@@ -2940,7 +2940,7 @@ win32-nat.o: win32-nat.c $(defs_h) $(fra
 	$(regcache_h) $(top_h) $(buildsym_h) $(symfile_h) $(objfiles_h) \
 	$(gdb_string_h) $(gdbthread_h) $(gdbcmd_h) $(exec_h) $(solist_h) \
 	$(solib_h) $(i386_tdep_h) $(i387_tdep_h) $(gdb_obstack_h) \
-	$(xml_support_h) $(i386_cygwin_tdep_h)
+	$(xml_support_h) $(i386_cygwin_tdep_h) $(gdb_stdint_h)
 win32-termcap.o: win32-termcap.c
 wrapper.o: wrapper.c $(defs_h) $(value_h) $(exceptions_h) $(wrapper_h) \
 	$(ui_out_h)
Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.142
diff -u -p -r1.142 win32-nat.c
--- gdb/win32-nat.c	2 Dec 2007 21:32:46 -0000	1.142
+++ gdb/win32-nat.c	2 Dec 2007 21:43:12 -0000
@@ -48,6 +48,7 @@
 #include "objfiles.h"
 #include "gdb_obstack.h"
 #include "gdb_string.h"
+#include "gdb_stdint.h"
 #include "gdbthread.h"
 #include "gdbcmd.h"
 #include <sys/param.h>
@@ -828,7 +829,8 @@ handle_output_debug_string (struct targe
   int retval = 0;
 
   if (!target_read_string
-    ((CORE_ADDR) current_event.u.DebugString.lpDebugStringData, &s, 1024,
0)
+	((CORE_ADDR) (uintptr_t)
current_event.u.DebugString.lpDebugStringData,
+	&s, 1024, 0)
       || !s || !*s)
     /* nothing to do */;
   else if (strncmp (s, _CYGWIN_SIGNAL_STRING, sizeof
(_CYGWIN_SIGNAL_STRING) - 1) != 0)
@@ -1022,7 +1024,8 @@ handle_exception (struct target_waitstat
 	   and will be sent as a cygwin-specific-signal.  So, ignore SEGVs
if they show up
 	   within the text segment of the DLL itself. */
 	char *fn;
-	bfd_vma addr = (bfd_vma)
current_event.u.Exception.ExceptionRecord.ExceptionAddress;
+	bfd_vma addr = (bfd_vma) (uintptr_t) current_event.u.Exception.
+
ExceptionRecord.ExceptionAddress;
 	if ((!cygwin_exceptions && (addr >= cygwin_load_start && addr <
cygwin_load_end))
 	    || (find_pc_partial_function (addr, &fn, NULL, NULL)
 		&& strncmp (fn, "KERNEL32!IsBad", strlen ("KERNEL32!IsBad"))
== 0))
@@ -1932,20 +1935,23 @@ win32_xfer_memory (CORE_ADDR memaddr, gd
 		   struct target_ops *target)
 {
   DWORD done = 0;
   if (write)
     {
       DEBUG_MEM (("gdb: write target memory, %d bytes at 0x%08lx\n",
-		  len, (DWORD) memaddr));
-      if (!WriteProcessMemory (current_process_handle, (LPVOID) memaddr,
our,
+		  len, (DWORD) (uintptr_t) memaddr));
+      if (!WriteProcessMemory (current_process_handle, 
+			       (LPVOID) (uintptr_t) memaddr, our,
 			       len, &done))
 	done = 0;
-      FlushInstructionCache (current_process_handle, (LPCVOID) memaddr,
len);
+      FlushInstructionCache (current_process_handle, 
+			     (LPCVOID) (uintptr_t) memaddr, len);
     }
   else
     {
       DEBUG_MEM (("gdb: read target memory, %d bytes at 0x%08lx\n",
-		  len, (DWORD) memaddr));
-      if (!ReadProcessMemory (current_process_handle, (LPCVOID) memaddr,
our,
+		  len, (DWORD) (uintptr_t) memaddr));
+      if (!ReadProcessMemory (current_process_handle, 
+			      (LPCVOID) (uintptr_t) memaddr, our,
 			      len, &done))
 	done = 0;
     }

Re: [RFA v3] Allow cygwin native to compile with --enable-64-bit-bfd

[Christopher Faylor]

On Mon, Dec 03, 2007 at 04:17:14PM +0100, Pierre Muller wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Daniel Jacobowitz
>> Sent: Sunday, December 02, 2007 5:00 AM
>> To: gdb-patches@sourceware.org
>> Cc: Pierre Muller
>> Subject: Re: [RFA v2] Allow cygwin native to compile with --enable-64-
>> bit-bfd
>> 
>> On Sat, Dec 01, 2007 at 09:43:11PM -0500, Christopher Faylor wrote:
>> > I'd like to get opinions from other maintainers on the use of a macro
>> > for this case.  I don't like seeing unexplained double casts like
>> this
>> > and I think a macro could make it clearer.
>> >
>> > Daniel were you implying that you would just tolerate a macro here or
>> > do you think it's an ok idea.
>> 
>> I'm very familiar with this idiom, so the double cast stands out when
>> I see it.  If there were a macro I'd have to go figure out whether it
>> was doing something more subtle.  I'd write it this way, but not
>> complain if someone else wrote it differently.
>> 
>> > >2007-11-28  Pierre Muller  <muller@ics.u-strasbg.fr>
>> > >
>> > >	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte long.
>> > >	Add "gdb_stdint.h" dependency required for uintptr_t type use.
>> 
>> Added includes -> update Makefile.in.  Tom's been working on automatic
>> dependencies for GCC.  Maybe we can do something similar for GDB
>> soon...
>
> I modified the patch to add the change to Makefile.in.
>
>
>  After checking again that there was
>no change to the testsuite. 
>
>OK to commit?

Ok.

cgf

RE: [RFA v3] Allow cygwin native to compile with --enable-64-bit-bfd

[Pierre Muller]

> >> > >2007-11-28  Pierre Muller  <muller@ics.u-strasbg.fr>
> >> > >
> >> > >	*win32-nat.c: Allow compilation if CORE_ADDR is 8 byte
> long.
> >> > >	Add "gdb_stdint.h" dependency required for uintptr_t type
> use.
> >>
> >> Added includes -> update Makefile.in.  Tom's been working on
> automatic
> >> dependencies for GCC.  Maybe we can do something similar for GDB
> >> soon...
> >
> > I modified the patch to add the change to Makefile.in.
> >
> >
> >  After checking again that there was
> >no change to the testsuite.
> >
> >OK to commit?
> 
> Ok.

Thanks, I just commit this,
after verifying that you can now create a cygwin gdb
with "--enable-targets=all --enable-64bit-bfd"
configure options.

Pierre Muller