This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] PR binutils/19020: objcopy interleave feature broken

On Mon, Sep 28, 2015 at 04:54:22PM -0700, H.J. Lu wrote:
> On Mon, Sep 28, 2015 at 02:25:02PM -0700, H.J. Lu wrote:
> > In objcopy, copy_object calls copy_section to copy contents of input
> > section to output section.  When --gap-fill= is used, objcopy extends
> > the size of output sectios to faill gaps between output sections with
> > gap fills.  In this case, we should set the output section size to the
> > input section size to avoid reading beypond the input section buffer
> > before calling copy_section and restores the output section size after
> > input sections have been copied.
> > 
> > OK for master?
> > 
> 
> A simpler patch.
> 
> 
> H.J.
> --
> In objcopy, copy_object calls copy_section to copy contents of input
> section to output section.  When --gap-fill= is used, objcopy extends
> the size of output sectios to fill gaps between output sections with
> gap fills.  In this case, we should set the output section size to the
> input section size to avoid reading beypond the input section buffer
> before calling copy_section and restores the output section size after
> input sections have been copied.
> 
> binutils/
> 
> 	PR binutils/19005
> 	* objcopy.c (copy_object): Adjust the output section size to
> 	skip gap fills between sections when copying from input sections
> 	to output sections.
> 

We can't use the output section size to copy input section since
--interleave will shrink the output section.  Instead, we change
bfd_convert_section_contents to return the updated output section
size.  When we do that, we don't need to adjust the output section
size to skip gap fills.

bfd/

	PR binutils/19020
	* bfd.c (bfd_convert_section_contents): Add ptr_size parameter.
	* bfd-in2.h: Regenerated.

binutils/

	PR binutils/19020
	* objcopy.c (copy_object): Don't adjust the output section size
	when copying from input sections.
	(copy_section): Use input section size for the copy.  Get the
	updated section size from bfd_convert_section_contents.

binutils/testsuite/

	PR binutils/19020
	* binutils-all/objcopy.exp: Run pr19020a and pr19020b.
	* lib/utils-lib.exp (run_dump_test): Support binary input.
	* binutils-all/pr19020.in: New file.
	* binutils-all/pr19020a.d: Likewise.
	* binutils-all/pr19020b.d: Likewise.
---
 bfd/ChangeLog                               |  7 +++++++
 bfd/bfd-in2.h                               |  3 ++-
 bfd/bfd.c                                   |  8 +++++---
 binutils/ChangeLog                          |  9 +++++++++
 binutils/objcopy.c                          | 27 +++++----------------------
 binutils/testsuite/ChangeLog                |  9 +++++++++
 binutils/testsuite/binutils-all/objcopy.exp |  2 ++
 binutils/testsuite/binutils-all/pr19020.in  |  1 +
 binutils/testsuite/binutils-all/pr19020a.d  |  9 +++++++++
 binutils/testsuite/binutils-all/pr19020b.d  |  9 +++++++++
 binutils/testsuite/lib/utils-lib.exp        | 16 ++++++++++------
 11 files changed, 68 insertions(+), 32 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/pr19020.in
 create mode 100644 binutils/testsuite/binutils-all/pr19020a.d
 create mode 100644 binutils/testsuite/binutils-all/pr19020b.d

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index d8068c4..bcbe0e1 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,10 @@
+2015-09-29  Andrew Stubbs  <ams@codesourcery.com>
+	    H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR binutils/19020
+	* bfd.c (bfd_convert_section_contents): Add ptr_size parameter.
+	* bfd-in2.h: Regenerated.
+
 2015-08-11  Peter Zotov  <whitequark@whitequark.org>
 
 	PR ld/18759
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index f06d76e..0d096f9 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..f68f665 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;
@@ -2264,5 +2265,6 @@ bfd_convert_section_contents (bfd *ibfd, sec_ptr isec, bfd *obfd,
       *ptr = contents;
     }
 
+  *ptr_size = size;
   return TRUE;
 }
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 8d2f984..16ed7c4 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,12 @@
+2015-09-29  Andrew Stubbs  <ams@codesourcery.com>
+	    H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR binutils/19020
+	* objcopy.c (copy_object): Don't adjust the output section size
+	when copying from input sections.
+	(copy_section): Use input section size for the copy.  Get the
+	updated section size from bfd_convert_section_contents.
+
 2015-09-29  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR binutils/19005
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index c94d515..2cd55fd 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -2212,24 +2212,7 @@ copy_object (bfd *ibfd, bfd *obfd, const bfd_arch_info_type *input_arch)
   bfd_map_over_sections (ibfd, copy_relocations_in_section, obfd);
 
   /* This has to happen after the symbol table has been set.  */
-  if (gap_fill_set)
-    {
-      /* Adjust the output section size to skip gap fills between
-	 sections.  */
-      c = bfd_count_sections (obfd);
-      for (i = 0; i < c; i++)
-	if (gaps[i] != 0)
-	  osections[i]->size -= gaps[i];
-    }
   bfd_map_over_sections (ibfd, copy_section, obfd);
-  if (gap_fill_set)
-    {
-      /* Restore the output section size for gap fills between
-	 sections.  */
-      for (i = 0; i < c; i++)
-	if (gaps[i] != 0)
-	  osections[i]->size += gaps[i];
-    }
 
   if (add_sections != NULL)
     {
@@ -3127,10 +3110,10 @@ copy_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
 
   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);
+     ELF classes of input and output aren't the same.  We can't use
+     the output section size since --interleave will shrink the output
+     section.   Size will be updated if the section is converted.   */
+  size = bfd_get_section_size (isection);
 
   if (bfd_get_section_flags (ibfd, isection) & SEC_HAS_CONTENTS
       && bfd_get_section_flags (obfd, osection) & SEC_HAS_CONTENTS)
@@ -3139,7 +3122,7 @@ copy_section (bfd *ibfd, sec_ptr isection, void *obfdarg)
 
       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);
diff --git a/binutils/testsuite/ChangeLog b/binutils/testsuite/ChangeLog
index 9efe4dc..f444dc7 100644
--- a/binutils/testsuite/ChangeLog
+++ b/binutils/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2015-09-29  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR binutils/19020
+	* binutils-all/objcopy.exp: Run pr19020a and pr19020b.
+	* lib/utils-lib.exp (run_dump_test): Support binary input.
+	* binutils-all/pr19020.in: New file.
+	* binutils-all/pr19020a.d: Likewise.
+	* binutils-all/pr19020b.d: Likewise.
+
 2015-08-11  Alan Modra  <amodra@gmail.com>
 
 	* binutils-all/strip-12.s: Align .bss section.
diff --git a/binutils/testsuite/binutils-all/objcopy.exp b/binutils/testsuite/binutils-all/objcopy.exp
index cb8e33b..bb33e4f 100644
--- a/binutils/testsuite/binutils-all/objcopy.exp
+++ b/binutils/testsuite/binutils-all/objcopy.exp
@@ -1022,6 +1022,8 @@ if [is_elf_format] {
 run_dump_test "copy-2"
 run_dump_test "copy-3"
 run_dump_test "copy-4"
+run_dump_test "pr19020a"
+run_dump_test "pr19020b"
 
 if [is_elf_format] {
     run_dump_test "strip-1"
diff --git a/binutils/testsuite/binutils-all/pr19020.in b/binutils/testsuite/binutils-all/pr19020.in
new file mode 100644
index 0000000..1656f92
--- /dev/null
+++ b/binutils/testsuite/binutils-all/pr19020.in
@@ -0,0 +1 @@
+abcdefgh
\ No newline at end of file
diff --git a/binutils/testsuite/binutils-all/pr19020a.d b/binutils/testsuite/binutils-all/pr19020a.d
new file mode 100644
index 0000000..71d5813
--- /dev/null
+++ b/binutils/testsuite/binutils-all/pr19020a.d
@@ -0,0 +1,9 @@
+#PROG: objcopy
+#source: pr19020.in
+#as: binary
+#objcopy: -O binary -I binary --pad-to=10 --gap-fill=65 --interleave=2 --interleave-width=1 --byte=0
+#objdump: -b binary -s
+
+#...
+Contents of section .data:
+ 0000 61636567 41414141 4141 +acegAAAAAA +
diff --git a/binutils/testsuite/binutils-all/pr19020b.d b/binutils/testsuite/binutils-all/pr19020b.d
new file mode 100644
index 0000000..41dc650
--- /dev/null
+++ b/binutils/testsuite/binutils-all/pr19020b.d
@@ -0,0 +1,9 @@
+#PROG: objcopy
+#source: pr19020.in
+#as: binary
+#objcopy: -O binary -I binary --pad-to=10 --gap-fill=65 --reverse-bytes=8
+#objdump: -b binary -s
+
+#...
+Contents of section .data:
+ 0000 68676665 64636261 4141 +hgfedcbaAA +
diff --git a/binutils/testsuite/lib/utils-lib.exp b/binutils/testsuite/lib/utils-lib.exp
index 9c24cf1..fd5b6f6 100644
--- a/binutils/testsuite/lib/utils-lib.exp
+++ b/binutils/testsuite/lib/utils-lib.exp
@@ -447,12 +447,16 @@ proc run_dump_test { name {extra_options {}} } {
 	set srcfile $srcdir/$subdir/$opts(source)
     }
 
-    set exec_output [binutils_assemble_flags ${srcfile} $tempfile $opts(as)]
-    if [string match "" $exec_output] then {
-	send_log "$exec_output\n"
-	verbose "$exec_output"
-	fail $testname
-	return
+    if { $opts(as) == "binary" } {
+	file copy -force ${srcfile} $tempfile
+    } else {
+	set exec_output [binutils_assemble_flags ${srcfile} $tempfile $opts(as)]
+	if [string match "" $exec_output] then {
+	    send_log "$exec_output\n"
+	    verbose "$exec_output"
+	    fail $testname
+	   return
+	}
     }
 
     set progopts1 $opts($program)
-- 
2.4.3
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]