Re: [PATCH] - objcopy --extract-symbol clears e_flags ELF header field

[Alan Modra <amodra at gmail dot com>]

On Wed, Oct 14, 2015 at 09:44:39PM +0200, Ronald Hoogenboom wrote:
> Or move the 2 lines to after the bfd_copy_private_bfd_data. So BFD needs to
> be adapted for all kinds of targets I never even heard about. There the test
> in the testsuite will be indispensable. Can you give me some pointers on how
> to get that done?

My advice about moving the tests to binutils/testsuite/ probably
wasn't so good.  On looking at objcopy.exp in detail, I see many of
the tests require a C compiler.  That means you need a whole lot more
infrastructure in order to test cross-binutils well.  So perhaps the
test is best left in ld/testsuite, but moved out of ld-elf.

Like this.  The new testcase shows that objcopy --extract-symbol
isn't working on PE, mips, mmix and some aout targets.

binutils/
	* objcopy.c (copy_object): Don't omit bfd_copy_private_bfd_data
	call when extract_symbol.
bfd/
	* elf32-v850.c (v850_elf_copy_private_bfd_data): Remove assertion
	that input and output .note.renesas sections are same size.
	Instead, only copy input to output if they are.
ld/testsuite/
	* config/default.exp (size): New global.
	* ld-elf/extract-symbol-1.s,
	* ld-elf/extract-symbol-1.ld,
	* ld-elf/extract-symbol-1sec.d,
	* ld-elf/extract-symbol-1sym.d: Delete.
	* ld-scripts/script.exp (extract_symbol_test): New.

diff --git a/bfd/elf32-v850.c b/bfd/elf32-v850.c
index c6a6d2f..4773324 100644
--- a/bfd/elf32-v850.c
+++ b/bfd/elf32-v850.c
@@ -2442,13 +2442,14 @@ v850_elf_copy_private_bfd_data (bfd * ibfd, bfd * obfd)
   if ((onotes = bfd_get_section_by_name (obfd, V850_NOTE_SECNAME)) == NULL)
     return TRUE;
 
-  if ((inotes = bfd_get_section_by_name (ibfd, V850_NOTE_SECNAME)) != NULL)
+  if ((inotes = bfd_get_section_by_name (ibfd, V850_NOTE_SECNAME)) == NULL)
+    return TRUE;
+
+  if (bfd_section_size (ibfd, inotes) == bfd_section_size (obfd, onotes))
     {
       bfd_byte * icont;
       bfd_byte * ocont;
 
-      BFD_ASSERT (bfd_section_size (ibfd, inotes) == bfd_section_size (obfd, onotes));
-
       if ((icont = elf_section_data (inotes)->this_hdr.contents) == NULL)
 	BFD_ASSERT (bfd_malloc_and_get_section (ibfd, inotes, & icont));
 
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 2cd55fd..324456a 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -2305,11 +2305,6 @@ copy_object (bfd *ibfd, bfd *obfd, const bfd_arch_info_type *input_arch)
 	}
     }
 
-  /* Do not copy backend data if --extract-symbol is passed; anything
-     that needs to look at the section contents will fail.  */
-  if (extract_symbol)
-    return TRUE;
-
   /* Allow the BFD backend to copy any private data it understands
      from the input BFD to the output BFD.  This is done last to
      permit the routine to look at the filtered symbol table, which is
diff --git a/ld/testsuite/config/default.exp b/ld/testsuite/config/default.exp
index 5b3e29f..310a3b2 100644
--- a/ld/testsuite/config/default.exp
+++ b/ld/testsuite/config/default.exp
@@ -49,6 +49,10 @@ if ![info exists strip] then {
     set strip [findfile $base_dir/../binutils/strip-new $base_dir/../binutils/strip-new [transform strip]]
 }
 
+if ![info exists size] then {
+    set size [findfile $base_dir/../binutils/size]
+}
+
 remote_exec host "mkdir -p tmpdir"
 
 # Make symlinks from tmpdir/ld to the linker and assembler in the
diff --git a/ld/testsuite/ld-elf/extract-symbol-1.ld b/ld/testsuite/ld-elf/extract-symbol-1.ld
deleted file mode 100644
index 53e95c6..0000000
--- a/ld/testsuite/ld-elf/extract-symbol-1.ld
+++ /dev/null
@@ -1,18 +0,0 @@
-ENTRY(_entry)
-PHDRS
-{
-  data PT_LOAD AT (0);
-}
-SECTIONS
-{
-  . = 0x10000;
-  .foo : { *(.foo) } :data
-
-  . = 0x20000;
-  .bar : { *(.bar) } :data
-
-  /DISCARD/ : { *(*) }
-
-  _entry = 0x30000;
-  linker_symbol = 0x40000;
-}
diff --git a/ld/testsuite/ld-elf/extract-symbol-1.s b/ld/testsuite/ld-elf/extract-symbol-1.s
deleted file mode 100644
index 0971500..0000000
--- a/ld/testsuite/ld-elf/extract-symbol-1.s
+++ /dev/null
@@ -1,15 +0,0 @@
-	.globl		B
-	.globl		C
-
-	.section	.foo,"awx",%progbits
-	.4byte		1,2,3,4
-B:
-	.4byte		5,6,7
-
-	.section	.bar,"ax",%nobits
-	.space		0x123
-C:
-	.space		0x302
-
-	.globl		D
-	.equ		D,0x12345678
diff --git a/ld/testsuite/ld-elf/extract-symbol-1sec.d b/ld/testsuite/ld-elf/extract-symbol-1sec.d
deleted file mode 100644
index 35ca898..0000000
--- a/ld/testsuite/ld-elf/extract-symbol-1sec.d
+++ /dev/null
@@ -1,15 +0,0 @@
-#name: --extract-symbol test 1 (sections)
-#source: extract-symbol-1.s
-#ld: -Textract-symbol-1.ld
-#objcopy_linked_file: --extract-symbol
-#objdump: --headers
-#xfail: "hppa*-*-*" "rx-*-*" "v850*-*-*"
-# FAILS on the RX because the linker has to set LMA == VMA for the Renesas loader.
-# FAILS on the V850 because an extra section - .note.renesas - is created.
-#...
-Sections:
- *Idx +Name +Size +VMA +LMA .*
- *0 +\.foo +0+ +0+10000 +0+10000 .*
- *CONTENTS, ALLOC, LOAD, CODE
- *1 +\.bar +0+ +0+20000 +0+10000 .*
- *ALLOC, READONLY, CODE
diff --git a/ld/testsuite/ld-elf/extract-symbol-1sym.d b/ld/testsuite/ld-elf/extract-symbol-1sym.d
deleted file mode 100644
index 4069191..0000000
--- a/ld/testsuite/ld-elf/extract-symbol-1sym.d
+++ /dev/null
@@ -1,17 +0,0 @@
-#name: --extract-symbol test 1 (symbols)
-#source: extract-symbol-1.s
-#ld: -Textract-symbol-1.ld
-#objcopy_linked_file: --extract-symbol
-#nm: -n
-#xfail: "hppa*-*-*"
-#...
-0*00010010 T B
-#...
-0*00020123 T C
-#...
-0*00030000 A _entry
-#...
-0*00040000 A linker_symbol
-#...
-0*12345678 A D
-#pass
\ No newline at end of file
diff --git a/ld/testsuite/ld-scripts/script.exp b/ld/testsuite/ld-scripts/script.exp
index 6ab04ec..b5f175f 100644
--- a/ld/testsuite/ld-scripts/script.exp
+++ b/ld/testsuite/ld-scripts/script.exp
@@ -107,6 +107,35 @@ proc check_script { } {
     }
 }
 
+proc extract_symbol_test { testfile testname } {
+    global objcopy
+    global nm
+    global size
+
+    set copyfile tmpdir/extract
+    set args "--extract-symbol $testfile $copyfile"
+    set exec_output [run_host_cmd $objcopy $args]
+    if ![string equal "" $exec_output] {
+	fail $testname
+	return
+    }
+
+    set orig_syms [run_host_cmd $nm $testfile]
+    set extract_syms [run_host_cmd $nm $copyfile]
+    if ![string equal $orig_syms $extract_syms] {
+	fail $testname
+	return
+    }
+
+    set exec_output [run_host_cmd $size $copyfile]
+    if ![regexp ".* 0\[	 \]+0\[	 \]+0\[	 \]+0\[	 \]+0\[	 \]+.*" $exec_output] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
 # PE targets need to set the image base to 0 to avoid complications from nm.
 set flags ""
 if {[istarget "*-*-pe*"] \
@@ -143,8 +172,10 @@ if ![ld_simple_link $ld tmpdir/script "$flags -T $srcdir/$subdir/memory.t tmpdir
 set testname "MEMORY with symbols"
 if ![ld_simple_link $ld tmpdir/script "$flags -defsym DATA_ORIGIN=0x1000 -defsym DATA_LENGTH=0x10000 -T $srcdir/$subdir/memory_sym.t tmpdir/script.o"] {
     fail $testname
+    untested "extract symbols"
 } else {
     check_script
+    extract_symbol_test tmpdir/script "extract symbols"
 }
 
 set test_script_list [lsort [glob $srcdir/$subdir/region-alias-*.t]]

-- 
Alan Modra
Australia Development Lab, IBM