Re: [committed, PATCH] Properly convert objects between different ELF classes

[Andrew Stubbs <ams at codesourcery dot com>]

On 25/09/15 00:44, Alan Modra wrote:
> OK.  The testsuite addition is OK too after building and testing a
> good selection of targets, and xfailing the inevitable breakage.  I
> use these:

After double-checking, I've discovered that I've misunderstood the code, 
and the input section's size is not updated by the conversion process 
(the conversion appears to affect local data only), so the patch would 
be broken for compressed data.

This patch should solve my problem without creating a new one.

As for the testcase, after tweaking the pad amount, it only fails on 6 
of the targets Alan listed, for which I've added xfails. I presume 
they're doing some conversion on the input section size such that the 
data size *should* be the output size (valgrind shows no issues).

OK?

Andrew

2015-09-25  Andrew Stubbs  <ams@codesourcery.com>

	bfd/
	* bfd.c (bfd_convert_section_contents): Add ptr_size parameter.
	* bfd-in2.h (bfd_convert_section_contents): Likewise.

	binutils/
	* objcopy.c (copy_section): Use input section size for the copy.

	binutils/testsuite/
	* binutils-all/objcopy.exp (use input sizes):  New test.
	* binutils-all/three-byte-text.s: New file.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index f06d76e..c35f148 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -6933,7 +6933,8 @@ bfd_size_type bfd_convert_section_size
    (bfd *ibfd, asection *isec, bfd *obfd, bfd_size_type size);
 
 bfd_boolean bfd_convert_section_contents
-   (bfd *ibfd, asection *isec, bfd *obfd, bfd_byte **ptr);
+   (bfd *ibfd, asection *isec, bfd *obfd, bfd_byte **ptr,
+    bfd_size_type *ptr_size);
 
 /* Extracted from archive.c.  */
 symindex bfd_get_next_mapent
diff --git a/bfd/bfd.c b/bfd/bfd.c
index 449dfe6..dc6fd59 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -2165,19 +2165,20 @@ FUNCTION
 
 SYNOPSIS
 	bfd_boolean bfd_convert_section_contents
-	  (bfd *ibfd, asection *isec, bfd *obfd, bfd_byte **ptr);
+	  (bfd *ibfd, asection *isec, bfd *obfd,
+	   bfd_byte **ptr, bfd_size_type *ptr_size);
 
 DESCRIPTION
 	Convert the contents, stored in @var{*ptr}, of the section
 	@var{isec} in input BFD @var{ibfd} to output BFD @var{obfd}
 	if needed.  The original buffer pointed to by @var{*ptr} may
 	be freed and @var{*ptr} is returned with memory malloc'd by this
-	function.
+	function, and the new size written to @var{ptr_size}.
 */
 
 bfd_boolean
 bfd_convert_section_contents (bfd *ibfd, sec_ptr isec, bfd *obfd,
-			      bfd_byte **ptr)
+			      bfd_byte **ptr, bfd_size_type *ptr_size)
 {
   bfd_byte *contents;
   bfd_size_type ihdr_size, ohdr_size, size;
@@ -2262,6 +2263,7 @@ bfd_convert_section_contents (bfd *ibfd, sec_ptr isec, bfd *obfd,
       memcpy (contents + ohdr_size, *ptr + ihdr_size, size - ohdr_size);
       free (*ptr);
       *ptr = contents;
+      *ptr_size = size;
     }
 
   return TRUE;
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index e4cb3e2..6e03029 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -3104,26 +3104,24 @@ copy_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
   bfd *obfd = (bfd *) obfdarg;
   struct section_list *p;
   sec_ptr osection;
-  bfd_size_type size;
 
   if (skip_section (ibfd, isection))
     return;
 
   osection = isection->output_section;
-  /* The output SHF_COMPRESSED section size is different from input if
-     ELF classes of input and output aren't the same.  We must use the
-     output section size here, which has been updated in setup_section
-     via bfd_convert_section_size.  */
-  size = bfd_get_section_size (osection);
 
   if (bfd_get_section_flags (ibfd, isection) & SEC_HAS_CONTENTS
       && bfd_get_section_flags (obfd, osection) & SEC_HAS_CONTENTS)
     {
       bfd_byte *memhunk = NULL;
+      bfd_size_type size;
+
+      /* This will be updated if the section is converted.  */
+      size = bfd_get_section_size (isection);
 
       if (!bfd_get_full_section_contents (ibfd, isection, &memhunk)
 	  || !bfd_convert_section_contents (ibfd, isection, obfd,
-					    &memhunk))
+					    &memhunk, &size))
 	{
 	  status = 1;
 	  bfd_nonfatal_message (NULL, ibfd, isection, NULL);
@@ -3188,6 +3186,7 @@ copy_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
 				   FALSE, SECTION_CONTEXT_SET_FLAGS)) != NULL
 	   && (p->flags & SEC_HAS_CONTENTS) != 0)
     {
+      bfd_size_type size = bfd_get_section_size (osection);
       void *memhunk = xmalloc (size);
 
       /* We don't permit the user to turn off the SEC_HAS_CONTENTS
diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index cb8e33b..173fd66 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -1095,3 +1095,21 @@ if [is_elf_format] {
     run_dump_test "exclude-1b"
 }
 run_dump_test "localize-hidden-2"
+
+# Section copies should copy more data than there is in the input
+# (i.e. read past end of buffer)
+# We can't test this directly without valgrind, but --reverse-bytes
+# ought to use the same value.
+if {[binutils_assemble $srcdir/$subdir/three-byte-text.s tmpdir/bintest.o]} then {
+    setup_xfail "alpha-dec-vms" "hppa-hp-hpux10" "ns32k-*-netbsd" "rx-*-elf" "tic4x-*-coff" "tic54x-*-coff"
+    set got [binutils_run $OBJCOPY "-O binary -j .text --pad-to=6 --reverse-bytes=6 tmpdir/bintest.o"]
+
+    if [regexp {cannot reverse bytes: length of section .text must be evenly divisible by 6} $got] then {
+	pass "objcopy (use input sizes)"
+    } else {
+	fail "objcopy (use input sizes)"
+    }
+} else {
+    perror "unresolved use input sizes"
+    unresolved "objcopy (use input sizes)"
+}
diff --git a/binutils/testsuite/binutils-all/three-byte-text.s b/binutils/testsuite/binutils-all/three-byte-text.s
index e69de29..05fdeff 100644
--- a/binutils/testsuite/binutils-all/three-byte-text.s
+++ b/binutils/testsuite/binutils-all/three-byte-text.s
@@ -0,0 +1,2 @@
+	.text
+	.byte 1,2,3