This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR ld/21233: Avoid sweeping forced-undefined symbols in section GC
- From: "Maciej W. Rozycki" <macro at imgtec dot com>
- To: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- Cc: Alan Modra <amodra at gmail dot com>, Nick Clifton <nickc at redhat dot com>, <binutils at sourceware dot org>
- Date: Wed, 5 Apr 2017 17:03:22 +0100
- Subject: Re: [PATCH] PR ld/21233: Avoid sweeping forced-undefined symbols in section GC
- Authentication-results: sourceware.org; auth=none
- References: <201704050113.v351DhHw028416@ignucius.se.axis.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