symfile.c has: /* Opaque data for load_section_callback. */ struct load_section_data { unsigned long load_offset; struct load_progress_data *progress_data; VEC(memory_write_request_s) *requests; }; static void load_section_callback (bfd *abfd, asection *asec, void *data) { struct memory_write_request *new_request; ... new_request->begin = bfd_section_lma (abfd, asec) + args->load_offset; ... void generic_load (char *args, int from_tty) { ... cbdata.load_offset = strtoul (argv[1], &endptr, 0); "unsigned long" is not wide enough in the cases of 32-bit gdb x 64-bit target, or LLP64 (like Windows 64-bit). load_offset should be ULONGEST or CORE_ADDR.
Kai mentions monitor.c has a similar issue in monitor_load(). Other xxx_load functions should be audited too.
CVSROOT: /cvs/src Module name: src Changes by: ktietz@sourceware.org 2013-02-19 18:31:50 Modified files: gdb : ChangeLog symfile.c Log message: PR gdb/15161 * symfile.c (load_section_data): Change type of load_offset to CORE_ADDR. (generic_load): User strtoulst instead of strtoul for conversion of load_offset. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15169&r2=1.15170 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/symfile.c.diff?cvsroot=src&r1=1.363&r2=1.364
CVSROOT: /cvs/src Module name: src Changes by: palves@sourceware.org 2013-02-19 19:27:22 Modified files: gdb : ChangeLog monitor.c Log message: Harmonize this monitor_load with generic_load. Harmonize this old-looking code with generic_load, which fixes several issues. 2013-02-19 Pedro Alves <palves@redhat.com> PR gdb/15161 Harmonize with generic_load. * monitor.c: Include "readline/readline.h". (monitor_load): Rename parameter 'file' to 'args'. Use build_argv instead of sscanf. Use CORE_ADDR/strtoulst instead of unsigned long/strtol for the 'load_offset' local. Error out if no argument is given or if too many arguments are given. Tilde expand the passed in file name. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15170&r2=1.15171 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/monitor.c.diff?cvsroot=src&r1=1.113&r2=1.114
CVSROOT: /cvs/src Module name: src Changes by: palves@sourceware.org 2013-02-19 20:52:58 Modified files: gdb/gdbserver : ChangeLog server.c Log message: gdbserver:server.c - use unpack_varlen_hex to extract hex numbers. Addresses, as most numbers in the RSP are hex encoded, with variable length (that just means the width isn't specified, and there's no top cap. So they should be extracted with unpack_varlen_hex. A couple spots in server.c are using strto(u)l, which doesn't work on LLP64 targets. This patch fixes it. Tested on x86_64 Fedora 17. 2013-02-19 Pedro Alves <palves@redhat.com> Kai Tietz <ktietz@redhat.com> PR gdb/15161 * server.c (handle_query) <CRC check>: Use unpack_varlen_hex instead of strtoul to extract address from packet. (process_serial_event) <'z'>: Likewise. Patches: http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/gdbserver/ChangeLog.diff?cvsroot=src&r1=1.681&r2=1.682 http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/gdbserver/server.c.diff?cvsroot=src&r1=1.184&r2=1.185
remote-mips.c:mips_load calls pmon_load_fast which has: static void pmon_load_fast (char *file) { ... printf_filtered ("%s\t: 0x%4x .. 0x%4x ", s->name, (unsigned int) s->vma, (unsigned int) (s->vma + bfd_get_section_size (s))); This printf truncates vma's to 32-bit.
I audited all target_ops::to_load implementations, and didn't see any other similar issue.
And looks weirdly buggy. "0x%4x" results in printing things like "filename<tab>: 0x 1 .. 0x 2 " It must be that "0x%04x" was meant, so we'd get "0x0001", "0x0002"...
Alright, I almost committed a patch for that, but I noticed that the "final" local seen on the patch is an "int", and following that local's usage we see lots of 32-bit assumption. So I'll just leave the code as is. commit 023172580ad3b79cec08ee48fa422ba4cbbcfaa5 Author: Pedro Alves <palves@redhat.com> Date: Tue Feb 19 20:59:31 2013 +0000 Another bit of gdb/15161 - remote-mips.c This printf truncates vma's to 32-bit. I don't know if this is used or reacheable with MIPS64 at all, but as long as I noticed it, better make it follow gdb's conventions. Actually, the existing code looks weirdly buggy. "0x%4x" results in printing things like "filename<tab>: 0x 1 .. 0x 2 " It must be that "0x%04x" was meant, so we'd get instead: "filename<tab>: 0x0001 .. 0x0002 " phex does that. Tested by building with --enable-targets=all. 2013-02-19 Pedro Alves <palves@redhat.com> * remote-mips.c (pmon_load_fast): Use phex with target's address width instead of "0x%4x" format for printing addresses. diff --git a/gdb/remote-mips.c b/gdb/remote-mips.c index e20a740..8457890 100644 --- a/gdb/remote-mips.c +++ b/gdb/remote-mips.c @@ -3373,6 +3373,7 @@ pmon_load_fast (char *file) int final = 0; int finished = 0; struct cleanup *cleanup; + int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8; buffer = (char *) xmalloc (MAXRECSIZE + 1); binbuf = (unsigned char *) xmalloc (BINCHUNK); @@ -3413,9 +3414,10 @@ pmon_load_fast (char *file) bintotal += bfd_get_section_size (s); final = (s->vma + bfd_get_section_size (s)); - printf_filtered ("%s\t: 0x%4x .. 0x%4x ", s->name, - (unsigned int) s->vma, - (unsigned int) (s->vma + bfd_get_section_size (s))); + printf_filtered ("%s\t: 0x%s .. 0x%s ", s->name, + phex (s->vma, addr_size), + phex (s->vma + bfd_get_section_size (s), addr_size)); + gdb_flush (gdb_stdout); /* Output the starting address. */
> I audited all target_ops::to_load implementations, and didn't see any other similar issue. Alright, marking fixed.