Re: [PATCH, binutils] Fix implib test failures

[Alan Modra <amodra at gmail dot com>]

On Wed, Jul 20, 2016 at 05:16:32PM +0100, Thomas Preudhomme wrote:
> On Tuesday 19 July 2016 16:34:26 Nick Clifton wrote:
> > Hi Thomas,
> > 
> > >> +setup_xfail "am33lin-*-*" "d30v-*-*" "dlx-*-*" "i960-*-*" "m68hc1x-*-*"
> > >> +setup_xfail "m88k-*-*" "pj-*-*" "score7-*-*" "sh64-*-*" "vxworks-*-*"
> > >> 
> > >>   There are other score targets apart from score7.  score-elf and
> > >>   score3-elf
> > >> 
> > >> for example.  How about score*-*-* instead ?
> > > 
> > > I did not know how to get the affected triplet. All I got was a list of
> > > bfd/elf*-*.c that do not define elf_backend_relocate_section. I use the
> > > the
> > > value for the second * as the machine bit of the triplet.
> > 
> > Right, and you skipped the elfxx-*.c targets that matched this test as well,
> > right.  I think that the test should be any elf*.c file that does not
> > define elf_backend_relocate section or reference bfd_elf_final_link and
> > which *does* include elfNN-target.h - ie one which creates an
> > elf_backend_data structure.
> > 
> > So I think that you can skip vxworks entirely here, since elf-vxworks.c is
> > just a support file for vxworks targets, and not a cpu type by itself.
> > (Similarly for am33lin which just #include's elf-m10300.c which then does
> > define elf_backend_relocate_section.
> > 
> > > Are there other
> > > triplet in this list you see as not being inclusive enough?
> > 
> > Nope.
> > 
> > > How can I find the
> > > list of triplet corresponding to a given bfd file?
> > 
> > Look in bfd/configure.ac.  Starting at around line 382 there is a list
> > of targets and the target specific files that they need.  A little bit
> > of searching can tell you which files are used by which targets.
> 
> Alright, I removed vxworks and am33lin and use score*-*-* as suggested. I also 
> removed m68hc1x (either elf32-m68hc11.c or bfd/elf32-m68hc12.c are used and 
> they define elf_backend_relocate_section). I'm still puzzled at score since I 
> only see one line for it which does not suggest me that there is several value 
> possible. I'm also unsure about sh64 whose sh64_elf32_vec variant does not 
> define elf_backend_relocate_section.
> 
> Alan, you seem to already have an automated build setup with all possible 
> targets. Would you mind letting me know if the attached updated patch fixes 
> everything?

No, it doesn't, because the setup_xfail is only effective for the
first of the two tests.  I see you're also having trouble determining
the list of targets to xfail, so I'll help out and fix this for you.

Here's what I'll commit, after running another regression test since I
made some last-minute tweaks.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index ddf29bf..b145feb 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,13 @@
+2016-07-21  Nick Clifton  <nickc@redhat.com>
+
+	* elf.c (_bfd_elf_filter_global_symbols): Skip local symbols.
+	(swap_out_syms): Return an error when not finding ELF output
+	section rather than asserting.
+
+2016-07-21  Thomas Preud'homme  <thomas.preudhomme@arm.com>
+
+	* elflink.c (elf_output_implib): Call bfd_set_error on no symbols.
+
 2016-07-20  John Baldwin  <jhb@FreeBSD.org>
 
 	* elf.c (elfcore_grok_freebsd_psinfo): Check for minimum note size
diff --git a/bfd/elf.c b/bfd/elf.c
index ba1774e..274cd53 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -3893,9 +3893,10 @@ _bfd_elf_filter_global_symbols (bfd *abfd, struct bfd_link_info *info,
 	continue;
 
       h = bfd_link_hash_lookup (info->hash, name, FALSE, FALSE, FALSE);
+      if (h == NULL)
+	continue;
       if (h->type != bfd_link_hash_defined && h->type != bfd_link_hash_defweak)
 	continue;
-
       if (h->linker_def || h->ldscript_def)
 	continue;
 
@@ -7643,7 +7644,9 @@ error_return:
 		     section of a symbol to be a section that is
 		     actually in the output file.  */
 		  sec2 = bfd_get_section_by_name (abfd, sec->name);
-		  if (sec2 == NULL)
+		  if (sec2 != NULL)
+		    shndx = _bfd_elf_section_from_bfd_section (abfd, sec2);
+		  if (shndx == SHN_BAD)
 		    {
 		      _bfd_error_handler (_("\
 Unable to find equivalent output section for symbol '%s' from section '%s'"),
@@ -7652,9 +7655,6 @@ Unable to find equivalent output section for symbol '%s' from section '%s'"),
 		      bfd_set_error (bfd_error_invalid_operation);
 		      goto error_return;
 		    }
-
-		  shndx = _bfd_elf_section_from_bfd_section (abfd, sec2);
-		  BFD_ASSERT (shndx != SHN_BAD);
 		}
 	    }
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index a994b83..5bc5740 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -11091,6 +11091,7 @@ elf_output_implib (bfd *abfd, struct bfd_link_info *info)
     symcount = _bfd_elf_filter_global_symbols (abfd, info, sympp, symcount);
   if (symcount == 0)
     {
+      bfd_set_error (bfd_error_no_symbols);
       (*_bfd_error_handler) (_("%B: no symbol found for import library"),
 			     implib_bfd);
       goto free_sym_buf;
diff --git a/ld/ChangeLog b/ld/ChangeLog
index a2493d0..ad26f62 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,14 @@
+2016-07-21  Alan Modra  <amodra@gmail.com>
+
+	* testsuite/lib/ld-lib.exp (run_ld_link_tests): Add optional
+	parameter to pass list of xfails.
+	* testsuite/ld-elf/elf.exp: Add xfails for implib tests.  Tidy
+	implib test formatting.  Don't set .data start address.
+	* testsuite/ld-elf/implib.s: Remove first .bss directive and
+	replace second one with equivalent .section directive.
+	* testsuite/ld-elf/empty-implib.out: Add expected final error.
+	* testsuite/ld-elf/implib.rd: Update.
+
 2016-07-20  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR ld/20376
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 832f313..7ae610e 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -136,21 +136,40 @@ foreach t $test_list {
     run_dump_test [file rootname $t]
 }
 
+# Targets using the generic linker backend don't support generating
+# an import library.
+set xfail_implib ""
+if { [istarget "d30v-*-*"]
+     || [istarget "dlx-*-*"]
+     || ([istarget "fr30-*-*"] && ![istarget "fr30-*-linux*"])
+     || [istarget "frv-*-*"]
+     || [istarget "ft32-*-*"]
+     || [istarget "i860-*-*"]
+     || [istarget "i960-*-*"]
+     || [istarget "iq2000-*-*"]
+     || [istarget "mn10200-*-*"]
+     || [istarget "moxie-*-*"]
+     || [istarget "msp430-*-*"]
+     || [istarget "mt-*-*"]
+     || [istarget "pj*-*-*"] } {
+    set xfail_implib "*-*-*"
+}
+
 # Check that the --out-implib option work correctly.
 run_ld_link_tests {
     {"Generate empty import library"
-     "--out-implib=tmpdir/implib.lib" ""
-     "--defsym NO_GLOBAL=1"
-     {implib.s}
-     {{ld empty-implib.out}}
-     "implib"}
+	"--out-implib=tmpdir/implib.lib" ""
+	"--defsym NO_GLOBAL=1"
+	{implib.s}
+	{{ld empty-implib.out}}
+	"implib"}
     {"Generate import library"
-     "-Tdata=0x1000 --out-implib=tmpdir/implib.lib" ""
-     ""
-     {implib.s}
-     {{readelf {-s tmpdir/implib.lib} implib.rd}}
-     "implib"}
-}
+	"--out-implib=tmpdir/implib.lib" ""
+	""
+	{implib.s}
+	{{readelf {-s tmpdir/implib.lib} implib.rd}}
+	"implib"}
+} $xfail_implib
 
 if { [istarget *-*-linux*]
      || [istarget *-*-nacl*]
diff --git a/ld/testsuite/ld-elf/empty-implib.out b/ld/testsuite/ld-elf/empty-implib.out
index b123064..f611eb7 100644
--- a/ld/testsuite/ld-elf/empty-implib.out
+++ b/ld/testsuite/ld-elf/empty-implib.out
@@ -1,2 +1,3 @@
 .*: .*: no symbol found for import library
 .*: .*: failed to generate import library
+.*: final link failed: No symbols
diff --git a/ld/testsuite/ld-elf/implib.rd b/ld/testsuite/ld-elf/implib.rd
index 9f854a5..6835f39 100644
--- a/ld/testsuite/ld-elf/implib.rd
+++ b/ld/testsuite/ld-elf/implib.rd
@@ -1,10 +1,10 @@
-File: tmpdir/implib.lib
+File: tmpdir/implib\.lib
 
-Symbol table '.symtab' contains 3 entries:
-   Num:    Value +Size Type    Bind   Vis      Ndx Name
-     0: [0-9a-f]+     0 NOTYPE  LOCAL  DEFAULT  UND 
-     1: 0+100[0-3]     1 OBJECT  GLOBAL DEFAULT  ABS exported1
-     2: 0+100[0-3]     1 OBJECT  GLOBAL DEFAULT  ABS exported2
+Symbol table '\.symtab' contains 3 entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+ +0: [0-9a-f]+ +0 NOTYPE +LOCAL +DEFAULT +UND 
+ +1: [0-9a-f]+ +1 OBJECT +GLOBAL DEFAULT +ABS exported1
+ +2: [0-9a-f]+ +1 OBJECT +GLOBAL DEFAULT +ABS exported2
 
 File: tmpdir/implib
 
diff --git a/ld/testsuite/ld-elf/implib.s b/ld/testsuite/ld-elf/implib.s
index a86a940..e4d527b 100644
--- a/ld/testsuite/ld-elf/implib.s
+++ b/ld/testsuite/ld-elf/implib.s
@@ -1,5 +1,4 @@
 .ifndef NO_GLOBAL
-	.bss
 	.comm	exported1,1
 
 	.data
@@ -10,7 +9,7 @@ exported2:
 	.byte	21
 .endif
 
-	.bss
+	.section ".bss", "aw", %nobits
 not_exported1:
 	.space	1
 	.size	not_exported1, 1
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index 62f9768..9cd1959 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -1159,7 +1159,9 @@ proc ar_simple_create { ar aropts target objects } {
 #     in all other cases, any output from the linker during linking is
 #     treated as a sign of an error and FAILs the test.
 #
-proc run_ld_link_tests { ldtests } {
+# args is an optional list of target triplets to be xfailed.
+#
+proc run_ld_link_tests { ldtests args } {
     global ld
     global as
     global nm
@@ -1188,6 +1190,10 @@ proc run_ld_link_tests { ldtests } {
 	    continue
 	}
 
+	foreach target $args {
+	    setup_xfail $target
+	}
+
 	set ld_options [lindex $testitem 1]
 	set ld_after [lindex $testitem 2]
 	set as_options [lindex $testitem 3]

-- 
Alan Modra
Australia Development Lab, IBM