[PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Siddhesh Poyarekar]

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

Hi,

The target_read_memory function pointer that
bfd_elf_bfd_from_remote_memory accepts current accepts int for length.
I have attached a patch which changes this argument to size_t. This
change is needed because I'm looking to make analogous changes in gdb to
ensure consistency of storage sizes passed across functions to ensure
that larger values are not truncated.

I could write a wrapper around or cast the function pointer explicitly,
but as Jan Kratochvil suggested, it would be cleaner to just make a
change in bfd since it should be taking size_t values anyway. The
conversation thread is here for reference:

http://sourceware.org/ml/gdb-patches/2012-05/msg00909.html

Attached are two patches, the first being changes to bfd and the second
is the change I need to make in gdb to make it work with the changes in
bfd.

Regards,
Siddhesh

bfd/ChangeLog:

2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
	of target_read_memory as size_t.
	* bfd-in2.h: Regenerate.
	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
	  argument of target_read_memory as size_t.
	(_bfd_elf32_bfd_from_remote_memory): Likewise.
	(_bfd_elf64_bfd_from_remote_memory): Likewise.
	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
	* enfcode.h (NAME): Likewise.

gdb/ChangeLog:

2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* target.c (target_read_memory): Make LEN argument as size_t.
	* target.h (target_read_memory): Likewise.

[-- Attachment #2: 0001-bfd-changes-for-target_read_memory.patch --]
[-- Type: text/x-patch, Size: 3176 bytes --]

diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index a61e721..9617428 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -698,7 +698,7 @@ extern int bfd_get_elf_phdrs
    the remote memory.  */
 extern bfd *bfd_elf_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, int len));
+   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, size_t len));
 
 extern struct bfd_section *_bfd_elf_tls_setup
   (bfd *, struct bfd_link_info *);
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index efd542f..585a54a 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -705,7 +705,7 @@ extern int bfd_get_elf_phdrs
    the remote memory.  */
 extern bfd *bfd_elf_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, int len));
+   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, size_t len));
 
 extern struct bfd_section *_bfd_elf_tls_setup
   (bfd *, struct bfd_link_info *);
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5426c93..fcfb42a 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1186,7 +1186,7 @@ struct elf_backend_data
      see elf.c, elfcode.h.  */
   bfd *(*elf_backend_bfd_from_remote_memory)
      (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-      int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, int len));
+      int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, size_t len));
 
   /* This function is used by `_bfd_elf_get_synthetic_symtab';
      see elf.c.  */
@@ -2260,10 +2260,10 @@ extern char *elfcore_write_register_note
 
 extern bfd *_bfd_elf32_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, int));
+   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t));
 extern bfd *_bfd_elf64_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, int));
+   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t));
 
 extern bfd_vma bfd_elf_obj_attr_size (bfd *);
 extern void bfd_elf_set_obj_attr_contents (bfd *, bfd_byte *, bfd_vma);
diff --git a/bfd/elf.c b/bfd/elf.c
index c5b04ac..7a06fb4 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9556,7 +9556,7 @@ bfd_elf_bfd_from_remote_memory
   (bfd *templ,
    bfd_vma ehdr_vma,
    bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, int))
+   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t))
 {
   return (*get_elf_backend_data (templ)->elf_backend_bfd_from_remote_memory)
     (templ, ehdr_vma, loadbasep, target_read_memory);
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index c985c63..2c8fe2b 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1615,7 +1615,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   (bfd *templ,
    bfd_vma ehdr_vma,
    bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, int))
+   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t))
 {
   Elf_External_Ehdr x_ehdr;	/* Elf file header, external form */
   Elf_Internal_Ehdr i_ehdr;	/* Elf file header, internal form */
-- 
1.7.7.6


[-- Attachment #3: 0002-gdb-changes-for-target_read_memory.patch --]
[-- Type: text/x-patch, Size: 1043 bytes --]

diff --git a/gdb/target.c b/gdb/target.c
index 151209e..91b4b47 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1756,7 +1756,7 @@ target_xfer_partial (struct target_ops *ops,
    it makes no progress, and then return how much was transferred).  */
 
 int
-target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len)
+target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, size_t len)
 {
   /* Dispatch to the topmost target, not the flattened current_target.
      Memory accesses check target->to_has_(all_)memory, and the
diff --git a/gdb/target.h b/gdb/target.h
index f80fba0..f3ef33a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -996,7 +996,8 @@ extern void target_dcache_invalidate (void);
 
 extern int target_read_string (CORE_ADDR, char **, int, int *);
 
-extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
+extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
+			       size_t len);
 
 extern int target_read_stack (CORE_ADDR memaddr, gdb_byte *myaddr, int len);
 
-- 
1.7.7.6

Re: [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Alan Modra]

On Mon, May 28, 2012 at 02:35:20PM +0530, Siddhesh Poyarekar wrote:
> 2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>
> 
> 	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
> 	of target_read_memory as size_t.
> 	* bfd-in2.h: Regenerate.
> 	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
> 	  argument of target_read_memory as size_t.
> 	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> 	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> 	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> 	* enfcode.h (NAME): Likewise.

Since Jan has already given the OK, and these functions are only used
by gdb, binutils maintainers hardly need to look at the patch.  OK
anyway, but please fix the ChangeLog

	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Siddhesh Poyarekar]

On Mon, 28 May 2012 20:33:13 +0930, Alan wrote:

> On Mon, May 28, 2012 at 02:35:20PM +0530, Siddhesh Poyarekar wrote:
> > 2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>
> > 
> > 	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN
> > argument of target_read_memory as size_t.
> > 	* bfd-in2.h: Regenerate.
> > 	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
> > 	  argument of target_read_memory as size_t.
> > 	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> > 	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> > 	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> > 	* enfcode.h (NAME): Likewise.
> 
> Since Jan has already given the OK, and these functions are only used
> by gdb, binutils maintainers hardly need to look at the patch.  OK
> anyway, but please fix the ChangeLog
> 
> 	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.
> 

Thanks, fixed ChangeLog for bfd below as per your suggestion.

Regards,
Siddhesh

2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
	of target_read_memory as size_t.
	* bfd-in2.h: Regenerate.
	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
	argument of target_read_memory as size_t.
	(_bfd_elf32_bfd_from_remote_memory): Likewise.
	(_bfd_elf64_bfd_from_remote_memory): Likewise.
	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
	* enfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.

Re: [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Jan Kratochvil]

On Mon, 28 May 2012 13:03:13 +0200, Alan Modra wrote:
> Since Jan has already given the OK, and these functions are only used
> by gdb, binutils maintainers hardly need to look at the patch.  OK
> anyway, but please fix the ChangeLog

I would like to make the function at least formally really working with the new
types, therefore the patch below.

OK from me for the gdb/ part.


Thanks,
Jan


bfd/
2012-05-28  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Make
	contents_size type bfd_size_type.

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index c985c63..1ff3f07 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1623,7 +1623,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   Elf_Internal_Phdr *i_phdrs, *last_phdr;
   bfd *nbfd;
   struct bfd_in_memory *bim;
-  int contents_size;
+  bfd_size_type contents_size;
   bfd_byte *contents;
   int err;
   unsigned int i;

[commit bfd+gdb] [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Jan Kratochvil]

On Mon, 28 May 2012 13:03:13 +0200, Alan Modra wrote:
> On Mon, May 28, 2012 at 02:35:20PM +0530, Siddhesh Poyarekar wrote:
> > 2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>
> > 
> > 	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
> > 	of target_read_memory as size_t.
> > 	* bfd-in2.h: Regenerate.
> > 	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
> > 	  argument of target_read_memory as size_t.
> > 	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> > 	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> > 	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> > 	* enfcode.h (NAME): Likewise.
> 
> Since Jan has already given the OK, and these functions are only used
> by gdb, binutils maintainers hardly need to look at the patch.  OK
> anyway, but please fix the ChangeLog

Checked in:
	http://sourceware.org/ml/binutils-cvs/2012-06/msg00005.html
	http://sourceware.org/ml/gdb-cvs/2012-06/msg00003.html

+ping for:
	http://sourceware.org/ml/binutils/2012-05/msg00396.html
	int -> bfd_size_type contents_size;


Thanks,
Jan

Re: [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Hans-Peter Nilsson]

> From: Siddhesh Poyarekar <siddhesh@redhat.com>

> The target_read_memory function pointer that
> bfd_elf_bfd_from_remote_memory accepts current accepts int for length.
> I have attached a patch which changes this argument to size_t. This
> change is needed because I'm looking to make analogous changes in gdb to
> ensure consistency of storage sizes passed across functions to ensure
> that larger values are not truncated.

There's always bfd_size_type, though I haven't checked if it
fits your needs.

> 2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>
> 
> 	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
> 	of target_read_memory as size_t.
> 	* bfd-in2.h: Regenerate.
> 	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
> 	  argument of target_read_memory as size_t.
> 	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> 	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> 	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> 	* enfcode.h (NAME): Likewise.

This caused failure to build for simulators for (at least) the
following targets:

cris-elf, frv-elf, h8300-elf, iq2000-elf, m32r-elf, mips-elf, and
mn10300-elf.

They fail building either of sim/common/cgen-utils.c,
sim/common/sim-command.c, sim/mips/interp.c, or
sim/common/nrun.c all due to lack of size_t definition; a
missing include of stddef.h before its use.

Should bfd.h include sysdep.h or what is missing?

For reference, the m32r-elf fail:

gcc -DHAVE_CONFIG_H   -DWITH_DEFAULT_MODEL='"m32r/d"'  -DPROFILE=1 -DWITH_PROFILE=-1   -DWITH_ALIGNMENT=STRICT_ALIGNMENT  -DWITH_TARGET_BYTE_ORDER=BIG_ENDIAN -DWITH_ENVIRONMENT=ALL_ENVIRONMENT   -DWITH_HOST_BYTE_ORDER=LITTLE_ENDIAN     -DWITH_SCACHE=16384       -DM32R_ELF   -I. -I/tmp/hpautotest-sim/src/sim/m32r -I../common -I/tmp/hpautotest-sim/src/sim/m32r/../common -I../../include -I/tmp/hpautotest-sim/src/sim/m32r/../../include -I../../bfd -I/tmp/hpautotest-sim/src/sim/m32r/../../bfd -I../../opcodes -I/tmp/hpautotest-sim/src/sim/m32r/../../opcodes  -g -O2 -c -o cgen-utils.o -MT cgen-utils.o -MMD -MP -MF .deps/cgen-utils.Tpo /tmp/hpautotest-sim/src/sim/m32r/../common/cgen-utils.c
In file included from /tmp/hpautotest-sim/src/sim/m32r/../common/cgen-utils.c:21:
../../bfd/bfd.h:708: error: expected declaration specifiers or '...' before 'size_t'

brgds, H-P

Re: [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Siddhesh Poyarekar]

On Fri, 1 Jun 2012 20:24:14 +0200, Hans-Peter wrote:
> There's always bfd_size_type, though I haven't checked if it
> fits your needs.

Should be OK as long as it always matches the size_t definition in the
stddef.h that gcc ships. They're both typically unsigned long, but if
that is so, then bfd_size_type should have been typedef'd to size_t
anyway.
 
> This caused failure to build for simulators for (at least) the
> following targets:
> 
> cris-elf, frv-elf, h8300-elf, iq2000-elf, m32r-elf, mips-elf, and
> mn10300-elf.
> 
> They fail building either of sim/common/cgen-utils.c,
> sim/common/sim-command.c, sim/mips/interp.c, or
> sim/common/nrun.c all due to lack of size_t definition; a
> missing include of stddef.h before its use.
> 
> Should bfd.h include sysdep.h or what is missing?

An stddef.h include in bfd.h should fix this. That or I can fix my
patch to use bfd_size_type provided its size is always equal to
size_t, so that the include is not needed. Which way would be
preferable?

Regards,
Siddhesh

Re: [PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Jan Kratochvil]

On Fri, 01 Jun 2012 20:24:14 +0200, Hans-Peter Nilsson wrote:
> There's always bfd_size_type, though I haven't checked if it
> fits your needs.

bfd_size_type seems to be always >= size_t so it should work.


> For reference, the m32r-elf fail:

I can confirm it with
	./configure --target=m32r-elf; make


On Fri, 01 Jun 2012 21:54:10 +0200, Siddhesh Poyarekar wrote:
> On Fri, 1 Jun 2012 20:24:14 +0200, Hans-Peter wrote:
> > There's always bfd_size_type, though I haven't checked if it
> > fits your needs.
> 
> Should be OK as long as it always matches the size_t definition in the
> stddef.h that gcc ships.

It is larger in some cases than size_t; but GDB can adapt, it is still better
than it was smaller before.


> They're both typically unsigned long, but if
> that is so, then bfd_size_type should have been typedef'd to size_t
> anyway.

Unfortunately bfd_size_type is not always type-compatible with size_t so
passing pointers to prototyped functions would not work.


> > Should bfd.h include sysdep.h or what is missing?

sysdep.h inclusion has caused problems which were avoided by
	Re: recent change broke gdb build
	http://sourceware.org/ml/binutils/2012-05/msg00224.html
but they can be hit in other cases like here, so I do not think bfd.h can
include sysdep.h.


> An stddef.h include in bfd.h should fix this.

stddef.h may not exist on the host system, it should include sysdep.h instead.


> That or I can fix my patch to use bfd_size_type provided its size is always
> equal to size_t, so that the include is not needed. Which way would be
> preferable?

I think the bfd/ part should use bfd_size_type and the gdb/ part should
continue to use size_t where possible, only in some few cases it needs to be
prototype-compatible with bfd/ it would use bfd_size_type.

Going to post a patch today as keeping HEAD broken is not good.


Thanks,
Jan

[patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Jan Kratochvil]

On Fri, 01 Jun 2012 22:31:23 +0200, Jan Kratochvil wrote:
> On Fri, 01 Jun 2012 21:54:10 +0200, Siddhesh Poyarekar wrote:
> > Should be OK as long as it always matches the size_t definition in the
> > stddef.h that gcc ships.
> 
> It is larger in some cases than size_t;

CFLAGS="-g -m32" ./configure --enable-64-bit-bfd i386-unknown-linux-gnu

symfile-mem.c: In function ‘symbol_file_add_from_memory’:
symfile-mem.c:80:7: error: passing argument 4 of ‘bfd_elf_bfd_from_remote_memory’ from incompatible pointer type [-Werror]
In file included from defs.h:101:0,
                 from symfile-mem.c:45:
../bfd/bfd.h:706:13: note: expected ‘int (*)(bfd_vma,  bfd_byte *, bfd_size_type)’ but argument is of type ‘int (*)(CORE_ADDR,  gdb_byte *, size_t)’


> Going to post a patch today as keeping HEAD broken is not good.

Below.


Sorry,
Jan



bfd/
2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>

    	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
    	of target_read_memory as bfd_size_type.
    	* bfd-in2.h: Regenerate.
    	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
    	argument of target_read_memory as size_t.
    	(_bfd_elf32_bfd_from_remote_memory): Likewise.
    	(_bfd_elf64_bfd_from_remote_memory): Likewise.
    	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
    	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.
    
gdb/
2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symfile-mem.c: 3 new gdb_static_assert for target_read_memory_bfd
	parameters.
	(target_read_memory_bfd): New function.
	(symbol_file_add_from_memory): Use it.

diff --git a/bfd/bfd-in.h b/bfd/bfd-in.h
index 9617428..5300b14 100644
--- a/bfd/bfd-in.h
+++ b/bfd/bfd-in.h
@@ -698,7 +698,8 @@ extern int bfd_get_elf_phdrs
    the remote memory.  */
 extern bfd *bfd_elf_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, size_t len));
+   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
+			      bfd_size_type len));
 
 extern struct bfd_section *_bfd_elf_tls_setup
   (bfd *, struct bfd_link_info *);
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 585a54a..8798ae4 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -705,7 +705,8 @@ extern int bfd_get_elf_phdrs
    the remote memory.  */
 extern bfd *bfd_elf_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, size_t len));
+   int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
+			      bfd_size_type len));
 
 extern struct bfd_section *_bfd_elf_tls_setup
   (bfd *, struct bfd_link_info *);
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index fcfb42a..6105287 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1186,7 +1186,8 @@ struct elf_backend_data
      see elf.c, elfcode.h.  */
   bfd *(*elf_backend_bfd_from_remote_memory)
      (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-      int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr, size_t len));
+      int (*target_read_memory) (bfd_vma vma, bfd_byte *myaddr,
+				 bfd_size_type len));
 
   /* This function is used by `_bfd_elf_get_synthetic_symtab';
      see elf.c.  */
@@ -2260,10 +2261,10 @@ extern char *elfcore_write_register_note
 
 extern bfd *_bfd_elf32_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t));
+   int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type));
 extern bfd *_bfd_elf64_bfd_from_remote_memory
   (bfd *templ, bfd_vma ehdr_vma, bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t));
+   int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type));
 
 extern bfd_vma bfd_elf_obj_attr_size (bfd *);
 extern void bfd_elf_set_obj_attr_contents (bfd *, bfd_byte *, bfd_vma);
diff --git a/bfd/elf.c b/bfd/elf.c
index 7a06fb4..d660d7f 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9556,7 +9556,7 @@ bfd_elf_bfd_from_remote_memory
   (bfd *templ,
    bfd_vma ehdr_vma,
    bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t))
+   int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type))
 {
   return (*get_elf_backend_data (templ)->elf_backend_bfd_from_remote_memory)
     (templ, ehdr_vma, loadbasep, target_read_memory);
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 2c8fe2b..cc55c86 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1615,7 +1615,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   (bfd *templ,
    bfd_vma ehdr_vma,
    bfd_vma *loadbasep,
-   int (*target_read_memory) (bfd_vma, bfd_byte *, size_t))
+   int (*target_read_memory) (bfd_vma, bfd_byte *, bfd_size_type))
 {
   Elf_External_Ehdr x_ehdr;	/* Elf file header, external form */
   Elf_Internal_Ehdr i_ehdr;	/* Elf file header, internal form */
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 91125e1..45d95a7 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -55,6 +55,25 @@
 #include "auxv.h"
 #include "elf/common.h"
 
+/* Verify parameters of target_read_memory_bfd and target_read_memory are
+   compatible.  */
+
+gdb_static_assert (sizeof (CORE_ADDR) == sizeof (bfd_vma));
+gdb_static_assert (sizeof (gdb_byte) == sizeof (bfd_byte));
+gdb_static_assert (sizeof (size_t) <= sizeof (bfd_size_type));
+
+/* Provide bfd/ compatible prototype for target_read_memory.  Casting would not
+   be enough as LEN width may differ.  */
+
+static int
+target_read_memory_bfd (bfd_vma memaddr, bfd_byte *myaddr, bfd_size_type len)
+{
+  /* MYADDR must be already allocated for the LEN size so it has to fit in
+     size_t.  */
+  gdb_assert ((size_t) len == len);
+
+  return target_read_memory (memaddr, myaddr, len);
+}
 
 /* Read inferior memory at ADDR to find the header of a loaded object file
    and read its in-core symbols out of inferior memory.  TEMPL is a bfd
@@ -77,7 +96,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
     error (_("add-symbol-file-from-memory not supported for this target"));
 
   nbfd = bfd_elf_bfd_from_remote_memory (templ, addr, &loadbase,
-					 target_read_memory);
+					 target_read_memory_bfd);
   if (nbfd == NULL)
     error (_("Failed to read a valid object file image from memory."));
 

Re: [patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Hans-Peter Nilsson]

> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Date: Fri, 1 Jun 2012 23:05:30 +0200

> bfd/
> 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
>     	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
>     	of target_read_memory as bfd_size_type.
>     	* bfd-in2.h: Regenerate.
>     	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
>     	argument of target_read_memory as size_t.

Nit; ITYM bfd_size_type --                ^^^^^^

brgds, H-P

Re: [patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Hans-Peter Nilsson]

> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Date: Fri, 1 Jun 2012 23:05:30 +0200

> bfd/
> 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
>     	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
>     	of target_read_memory as bfd_size_type.
>     	* bfd-in2.h: Regenerate.
>     	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
>     	argument of target_read_memory as size_t.
(..."as bfd_size_type.")

>     	(_bfd_elf32_bfd_from_remote_memory): Likewise.
>     	(_bfd_elf64_bfd_from_remote_memory): Likewise.
>     	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
>     	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.
>     
> gdb/
> 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* symfile-mem.c: 3 new gdb_static_assert for target_read_memory_bfd
> 	parameters.
> 	(target_read_memory_bfd): New function.
> 	(symbol_file_add_from_memory): Use it.

If this is ready to commit and you don't have the time today,
just say the word.

brgds, H-P

Re: [patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Jan Kratochvil]

On Mon, 04 Jun 2012 07:10:18 +0200, Hans-Peter Nilsson wrote:
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > Date: Fri, 1 Jun 2012 23:05:30 +0200
> 
> > bfd/
> > 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> >     	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
> >     	of target_read_memory as bfd_size_type.
> >     	* bfd-in2.h: Regenerate.
> >     	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
> >     	argument of target_read_memory as size_t.
> (..."as bfd_size_type.")
> 
> >     	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> >     	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> >     	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> >     	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.
> >     
> > gdb/
> > 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* symfile-mem.c: 3 new gdb_static_assert for target_read_memory_bfd
> > 	parameters.
> > 	(target_read_memory_bfd): New function.
> > 	(symbol_file_add_from_memory): Use it.
> 
> If this is ready to commit and you don't have the time today,
> just say the word.

I believe I need bfd/ approval by Alan or other bfd/ maintainer.


Thanks,
Jan

Re: [patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Hans-Peter Nilsson]

> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Date: Mon, 4 Jun 2012 07:24:38 +0200

> On Mon, 04 Jun 2012 07:10:18 +0200, Hans-Peter Nilsson wrote:
> > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > Date: Fri, 1 Jun 2012 23:05:30 +0200
> > 
> > > bfd/
> > > 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > > 
> > >     	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
> > >     	of target_read_memory as bfd_size_type.
> > >     	* bfd-in2.h: Regenerate.
> > >     	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
> > >     	argument of target_read_memory as size_t.
> > (..."as bfd_size_type.")
> > 
> > >     	(_bfd_elf32_bfd_from_remote_memory): Likewise.
> > >     	(_bfd_elf64_bfd_from_remote_memory): Likewise.
> > >     	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
> > >     	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.
> > >     
> > > gdb/
> > > 2012-06-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > > 
> > > 	* symfile-mem.c: 3 new gdb_static_assert for target_read_memory_bfd
> > > 	parameters.
> > > 	(target_read_memory_bfd): New function.
> > > 	(symbol_file_add_from_memory): Use it.
> > 
> > If this is ready to commit and you don't have the time today,
> > just say the word.
> 
> I believe I need bfd/ approval by Alan or other bfd/ maintainer.

(I take it all necessary testing is done, then.)

If it'd been me, I'd interpret Alan's: "since Jan has already
given the OK, and these functions are only used by gdb, binutils
maintainers hardly need to look at the patch" (regarding the
earlier corresponding breaking change to size_t) as an intent of
delegation of these bits, but that may be a bit wild.  In the
meantime, there's a broken tree.

brgds, H-P

Re: [patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Alan Modra]

On Mon, Jun 04, 2012 at 08:22:52AM +0200, Hans-Peter Nilsson wrote:
> > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > I believe I need bfd/ approval by Alan or other bfd/ maintainer.

I doubt any of the binutils maintainers would object to gdb
maintainers making fixes to parts of bfd intimately related to gdb.
Just call it obvious and commit.  ;)

-- 
Alan Modra
Australia Development Lab, IBM

[commit] [patch#2] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Jan Kratochvil]

On Mon, 04 Jun 2012 14:11:09 +0200, Alan Modra wrote:
> On Mon, Jun 04, 2012 at 08:22:52AM +0200, Hans-Peter Nilsson wrote:
> > > From: Jan Kratochvil <jan.kratochvil@redhat.com>
> > > I believe I need bfd/ approval by Alan or other bfd/ maintainer.
> 
> I doubt any of the binutils maintainers would object to gdb
> maintainers making fixes to parts of bfd intimately related to gdb.
> Just call it obvious and commit.  ;)

Therefore checked in:
	http://sourceware.org/ml/binutils-cvs/2012-06/msg00014.html
	http://sourceware.org/ml/gdb-cvs/2012-06/msg00017.html


Sorry,
Jan


bfd/
	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
	of target_read_memory as bfd_size_type.
	* bfd-in2.h: Regenerate.
	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
	argument of target_read_memory as bfd_size_type.
	(_bfd_elf32_bfd_from_remote_memory): Likewise.
	(_bfd_elf64_bfd_from_remote_memory): Likewise.
	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
	* elfcode.h (NAME(_bfd_elf,bfd_from_remote_memory)): Likewise.
	
gdb/
	* symfile-mem.c: 3 new gdb_static_assert for target_read_memory_bfd
	parameters.
	(target_read_memory_bfd): New function.
	(symbol_file_add_from_memory): Use it.

[PATCH] bfd: Use size_t for length argument totarget_read_memory function passed into bfd_elf_bfd_from_remote_memory

[Siddhesh Poyarekar]

Hi,

The target_read_memory function pointer that
bfd_elf_bfd_from_remote_memory accepts current accepts int for length.
I have attached a patch which changes this argument to size_t. This
change is needed because I'm looking to make analogous changes in gdb to
ensure consistency of storage sizes passed across functions to ensure
that larger values are not truncated.

I could write a wrapper around or cast the function pointer explicitly,
but as Jan Kratochvil suggested, it would be cleaner to just make a
change in bfd since it should be taking size_t values anyway. The
conversation thread is here for reference:

http://sourceware.org/ml/gdb-patches/2012-05/msg00909.html

Attached are two patches, the first being changes to bfd and the second
is the change I need to make in gdb to make it work with the changes in
bfd.

Regards,
Siddhesh

bfd/ChangeLog:

2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* bfd-in.h (bfd_elf_bfd_from_remote_memory): Make LEN argument
	of target_read_memory as size_t.
	* bfd-in2.h: Regenerate.
	* elf-bfd.h (elf_backend_bfd_from_remote_memory): Make LEN
	  argument of target_read_memory as size_t.
	(_bfd_elf32_bfd_from_remote_memory): Likewise.
	(_bfd_elf64_bfd_from_remote_memory): Likewise.
	* elf.c (bfd_elf_bfd_from_remote_memory): Likewise.
	* enfcode.h (NAME): Likewise.

gdb/ChangeLog:

2012-05-28  Siddhesh Poyarekar  <siddhesh@redhat.com>

	* target.c (target_read_memory): Make LEN argument as size_t.
	* target.h (target_read_memory): Likewise.