Re: [PATCH] PR ld/21233: Avoid sweeping forced-undefined symbols in section GC

["Maciej W. Rozycki" <macro@imgtec.com>]

On Wed, 5 Apr 2017, Hans-Peter Nilsson wrote:

> > 2. File a PR referring to commit d968975277ba and its discussion and KFAIL 
> >    the affected test cases for the problematic targets.
> 
> (or rather xfail; I don't know if there's a way to kfail with
> run_ld_link_tests and I don't think we use that in binutils.)

 Of course there is, see commit 3807734dbe48 for a somewhat non-trivial 
example.

 AFAIK XFAIL is unsuitable as it marks a problem with the test environment 
rather than a bug in the component being tested.  KFAIL OTOH forces you to 
create a PR, which prompts you to have the issue recorded in the bug 
tracker (also giving a chance for someone else to pick it up) rather than 
papered over and then forgotten.

 KFAIL's usage is indeed much more prominent in GDB than in binutils.

> I'm not a general maintainer, but FWIW my preference would have
> been this, to xfail the failing parts and also add affected
> maintainers on the ticket.

 On second thoughts there can be multiple bugs here, quite likely one or 
more per BFD backend.  So it doesn't really scale well.  And any active 
maintainer will notice the regression in their routine runs; XFAIL/KFAIL 
may OTOH be missed (my test scripts certainly do not pay attention to 
these on the basis of them being expected/known; I'd have to go through 
full logs to catch them).

> To those interested: the run_ld_link_tests source shows how to
> add xfails for a target or use this as an example.  (Not my
> preferred test-driver function, I prefer to iterate on
> run_dump_test *.d files; with a driver in place sometimes I only
> have to add that one file with a descriptive comment inside.)

 I prefer `run_dump_test' too where feasible, but it is not suitable for 
some test cases, not at least without bending backwards.

> As a sanity check, I made sure mips-linux still passed all
> tests.

 Thanks.

> diff --git a/ld/testsuite/ld-elf/shared.exp b/ld/testsuite/ld-elf/shared.exp
> index be30ec0..291f9e1 100644
> --- a/ld/testsuite/ld-elf/shared.exp
> +++ b/ld/testsuite/ld-elf/shared.exp
> @@ -152,7 +153,7 @@ if { [check_gc_sections_available] } {
>  	    "tmpdir/libpr21233.so" "" \
>  	    {pr21233.s} \
>  	    {{readelf --dyn-syms pr21233.sd}} \
> -	    "pr21233-3"]]
> +	     "pr21233-3"]] "cris*-*-*"

 Indentation issue here.

  Maciej