Re: [PATCH] fix build failure for win64, revise some format strings

Ozkan Sezer
2010-07-15T18:55:15Z

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

PING
Can someone please review this patch? (Win64 fails to build for a week.)

On Mon, Jul 12, 2010 at 2:51 PM, Ozkan Sezer <sezeroz@gmail.com> wrote:
> On Mon, Jul 12, 2010 at 1:14 PM, Ozkan Sezer <sezeroz@gmail.com> wrote:
>> Hi:
>>
>> The current gdb cvs fails building for win64:
>>
>> cc1: warnings being treated as errors
>> ../../gdb-cvs/gdb/target.c: In function
>> 'debug_to_can_accel_watchpoint_condition':
>> ../../gdb-cvs/gdb/target.c:3335: error: cast from pointer to integer
>> of different size
>> ../../gdb-cvs/gdb/target.c: In function 'debug_to_insert_watchpoint':
>> ../../gdb-cvs/gdb/target.c:3424: error: cast from pointer to integer
>> of different size
>> ../../gdb-cvs/gdb/target.c: In function 'debug_to_remove_watchpoint':
>> ../../gdb-cvs/gdb/target.c:3439: error: cast from pointer to integer
>> of different size
>>
>> The warnings are due to the unsigned long casts in gdb/target.c
>> with assumption of LP64 behavior which isn't true for wih64 (LLP64).
>> There are even more unsigned long casts there which aren't
>> causing warnings but still truncating the value.
>>
>> I thought that it would be best to solve this using inttypes.h macros.
>> An initial patch is attached. Since gdb already uses gnulib for stdint,
>> it can also use inttypes.
>>
>> In the patch, I also touched gdbserver/server.c in order to fix
>> another format string issue with windows:
>>
>> ../../../gdb-cvs/gdb/gdbserver/server.c: In function 'handle_query':
>> ../../../gdb-cvs/gdb/gdbserver/server.c:1542: warning: unknown
>> conversion type character 'l' in format
>> ../../../gdb-cvs/gdb/gdbserver/server.c:1542: warning: too many
>> arguments for format
>> ../../../gdb-cvs/gdb/gdbserver/server.c:1566: warning: unknown
>> conversion type character 'l' in format
>> ../../../gdb-cvs/gdb/gdbserver/server.c:1566: warning: too many
>> arguments for format
>>
>> These warnings are due to the fact that M$ doesn't support %ll.
>> Fixed by using %I64d with _WIN32 ifdefs.
>>
>> Patch was test by compiling gdb for x86_64-linux and for x86_64-w64-mingw32
>> where both targets already provide inttypes.h.
>>
>>        * target.c: Include inttypes.h.
>>        (debug_to_insert_breakpoint): Remove unnecessary unsigned long casts
>>        from int type variables. For pointers, cast to uintptr_t instead of
>>        unsigned long and use the PRIuPTR macro from inttypes.h instead of %ld.
>>        (debug_to_remove_breakpoint): Likewise.
>>        (debug_to_can_use_hw_breakpoint): Likewise.
>>        (debug_to_region_ok_for_hw_watchpoint): Likewise.
>>        (debug_to_can_accel_watchpoint_condition): Likewise.
>>        (debug_to_stopped_by_watchpoint): Likewise.
>>        (debug_to_stopped_data_address): Likewise.
>>        (debug_to_watchpoint_addr_within_range): Likewise.
>>        (debug_to_insert_hw_breakpoint): Likewise.
>>        (debug_to_remove_hw_breakpoint): Likewise.
>>        (debug_to_insert_watchpoint): Likewise.
>>        (debug_to_remove_watchpoint): Likewise.
>>
>>        gdbserver/server.c (handle_query): For windows, Use %I64d instead of
>>        %lld in the sprintf format string.
>>
>> Note: if this patch is OK'ed, someone with an experience with gnulib should
>> import inttypes into gdb (I don't feel at home with gnulib.)
>>
>> Regards.
>>
>> --
>> Ozkan
>>
>
> Just noticed that the patch had intptr_t (as a typo) instead of uintptr_t
> in a few places.  Fixed one attached.
>
> --
> Ozkan
>

[-- Attachment #2: gdb_pformat.diff --]
[-- Type: application/octet-stream, Size: 7791 bytes --]

* target.c: Include inttypes.h.
(debug_to_insert_breakpoint): Remove unnecessary unsigned long casts
from int type variables. For pointers, cast to uintptr_t instead of
unsigned long and use the PRIuPTR macro from inttypes.h instead of %ld.
(debug_to_remove_breakpoint): Likewise.
(debug_to_can_use_hw_breakpoint): Likewise.
(debug_to_region_ok_for_hw_watchpoint): Likewise.
(debug_to_can_accel_watchpoint_condition): Likewise.
(debug_to_stopped_by_watchpoint): Likewise.
(debug_to_stopped_data_address): Likewise.
(debug_to_watchpoint_addr_within_range): Likewise.
(debug_to_insert_hw_breakpoint): Likewise.
(debug_to_remove_hw_breakpoint): Likewise.
(debug_to_insert_watchpoint): Likewise.
(debug_to_remove_watchpoint): Likewise.

* gdbserver/server.c (handle_query): For windows, Use %I64d instead of
%lld in the sprintf format string.

Index: gdb/target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.259
diff -u -p -r1.259 target.c
--- gdb/target.c 7 Jul 2010 16:15:17 -0000 1.259
+++ gdb/target.c 12 Jul 2010 07:20:37 -0000
@@ -22,6 +22,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */

#include "defs.h"
+#include <inttypes.h>
#include <errno.h>
#include "gdb_string.h"
#include "target.h"
@@ -3270,9 +3271,8 @@ debug_to_insert_breakpoint (struct gdbar
retval = debug_target.to_insert_breakpoint (gdbarch, bp_tgt);

fprintf_unfiltered (gdb_stdlog,
- "target_insert_breakpoint (0x%lx, xxx) = %ld\n",
- (unsigned long) bp_tgt->placed_address,
- (unsigned long) retval);
+ "target_insert_breakpoint (0x%" PRIuPTR ", xxx) = %d\n",
+ (uintptr_t) bp_tgt->placed_address, retval);
return retval;
}

@@ -3285,9 +3285,8 @@ debug_to_remove_breakpoint (struct gdbar
retval = debug_target.to_remove_breakpoint (gdbarch, bp_tgt);

fprintf_unfiltered (gdb_stdlog,
- "target_remove_breakpoint (0x%lx, xxx) = %ld\n",
- (unsigned long) bp_tgt->placed_address,
- (unsigned long) retval);
+ "target_remove_breakpoint (0x%" PRIuPTR ", xxx) = %d\n",
+ (uintptr_t) bp_tgt->placed_address, retval);
return retval;
}

@@ -3299,11 +3298,8 @@ debug_to_can_use_hw_breakpoint (int type
retval = debug_target.to_can_use_hw_breakpoint (type, cnt, from_tty);

fprintf_unfiltered (gdb_stdlog,
- "target_can_use_hw_breakpoint (%ld, %ld, %ld) = %ld\n",
- (unsigned long) type,
- (unsigned long) cnt,
- (unsigned long) from_tty,
- (unsigned long) retval);
+ "target_can_use_hw_breakpoint (%d, %d, %d) = %d\n",
+ type, cnt, from_tty, retval);
return retval;
}

@@ -3315,10 +3311,9 @@ debug_to_region_ok_for_hw_watchpoint (CO
retval = debug_target.to_region_ok_for_hw_watchpoint (addr, len);

fprintf_unfiltered (gdb_stdlog,
- "target_region_ok_for_hw_watchpoint (%ld, %ld) = 0x%lx\n",
- (unsigned long) addr,
- (unsigned long) len,
- (unsigned long) retval);
+ "target_region_ok_for_hw_watchpoint (%" PRIuPTR \
+ ", %d) = 0x%" PRIuPTR "\n",
+ (uintptr_t) addr, len, (uintptr_t) retval);
return retval;
}

@@ -3331,9 +3326,9 @@ debug_to_can_accel_watchpoint_condition
retval = debug_target.to_can_accel_watchpoint_condition (addr, len, rw, cond);

fprintf_unfiltered (gdb_stdlog,
- "target_can_accel_watchpoint_condition (0x%lx, %d, %d, 0x%lx) = %ld\n",
- (unsigned long) addr, len, rw, (unsigned long) cond,
- (unsigned long) retval);
+ "target_can_accel_watchpoint_condition (0x%" PRIuPTR \
+ ", %d, %d, 0x%" PRIuPTR ") = %d\n",
+ addr, len, rw, (uintptr_t) cond, retval);
return retval;
}

@@ -3345,8 +3340,7 @@ debug_to_stopped_by_watchpoint (void)
retval = debug_target.to_stopped_by_watchpoint ();

fprintf_unfiltered (gdb_stdlog,
- "target_stopped_by_watchpoint () = %ld\n",
- (unsigned long) retval);
+ "target_stopped_by_watchpoint () = %d\n", retval);
return retval;
}

@@ -3358,9 +3352,8 @@ debug_to_stopped_data_address (struct ta
retval = debug_target.to_stopped_data_address (target, addr);

fprintf_unfiltered (gdb_stdlog,
- "target_stopped_data_address ([0x%lx]) = %ld\n",
- (unsigned long)*addr,
- (unsigned long)retval);
+ "target_stopped_data_address ([0x%" PRIuPTR "]) = %d\n",
+ (uintptr_t)*addr, retval);
return retval;
}

@@ -3375,9 +3368,9 @@ debug_to_watchpoint_addr_within_range (s
start, length);

fprintf_filtered (gdb_stdlog,
- "target_watchpoint_addr_within_range (0x%lx, 0x%lx, %d) = %d\n",
- (unsigned long) addr, (unsigned long) start, length,
- retval);
+ "target_watchpoint_addr_within_range (0x%" PRIuPTR \
+ ", 0x%" PRIuPTR ", %d) = %d\n",
+ (uintptr_t) addr, (uintptr_t) start, length, retval);
return retval;
}

@@ -3390,9 +3383,9 @@ debug_to_insert_hw_breakpoint (struct gd
retval = debug_target.to_insert_hw_breakpoint (gdbarch, bp_tgt);

fprintf_unfiltered (gdb_stdlog,
- "target_insert_hw_breakpoint (0x%lx, xxx) = %ld\n",
- (unsigned long) bp_tgt->placed_address,
- (unsigned long) retval);
+ "target_insert_hw_breakpoint (0x%" PRIuPTR \
+ ", xxx) = %d\n",
+ (uintptr_t) bp_tgt->placed_address, retval);
return retval;
}

@@ -3405,9 +3398,9 @@ debug_to_remove_hw_breakpoint (struct gd
retval = debug_target.to_remove_hw_breakpoint (gdbarch, bp_tgt);

fprintf_unfiltered (gdb_stdlog,
- "target_remove_hw_breakpoint (0x%lx, xxx) = %ld\n",
- (unsigned long) bp_tgt->placed_address,
- (unsigned long) retval);
+ "target_remove_hw_breakpoint (0x%" PRIuPTR \
+ ", xxx) = %d\n",
+ (uintptr_t) bp_tgt->placed_address, retval);
return retval;
}

@@ -3420,9 +3413,10 @@ debug_to_insert_watchpoint (CORE_ADDR ad
retval = debug_target.to_insert_watchpoint (addr, len, type, cond);

fprintf_unfiltered (gdb_stdlog,
- "target_insert_watchpoint (0x%lx, %d, %d, 0x%ld) = %ld\n",
- (unsigned long) addr, len, type, (unsigned long) cond,
- (unsigned long) retval);
+ "target_insert_watchpoint (0x%" PRIuPTR \
+ ", %d, %d, 0x%" PRIuPTR ") = %d\n",
+ (uintptr_t) addr, len, type, (uintptr_t) cond,
+ retval);
return retval;
}

@@ -3435,9 +3429,9 @@ debug_to_remove_watchpoint (CORE_ADDR ad
retval = debug_target.to_remove_watchpoint (addr, len, type, cond);

fprintf_unfiltered (gdb_stdlog,
- "target_remove_watchpoint (0x%lx, %d, %d, 0x%ld) = %ld\n",
- (unsigned long) addr, len, type, (unsigned long) cond,
- (unsigned long) retval);
+ "target_remove_watchpoint (0x%" PRIuPTR ", %d, %d, 0x%" PRIuPTR \
+ ") = %d\n",
+ (uintptr_t) addr, len, type, (uintptr_t) cond, retval);
return retval;
}

Index: gdb/gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.125
diff -u -p -r1.125 server.c
--- gdb/gdbserver/server.c 7 Jul 2010 16:14:04 -0000 1.125
+++ gdb/gdbserver/server.c 12 Jul 2010 09:50:13 -0000
@@ -1539,7 +1539,11 @@ handle_query (char *own_buf, int packet_

if (err == 0)
{
+#ifdef _WIN32
+ sprintf (own_buf, "%I64d", address);
+#else
sprintf (own_buf, "%llx", address);
+#endif
return;
}
else if (err > 0)
@@ -1563,7 +1567,11 @@ handle_query (char *own_buf, int packet_
n = (*the_target->get_tib_address) (ptid, &tlb);
if (n == 1)
{
+#ifdef _WIN32
+ sprintf (own_buf, "%I64d", tlb);
+#else
sprintf (own_buf, "%llx", tlb);
+#endif
return;
}
else if (n == 0)