Re: Committed: fix for PR ld/16044: elf32-cris.c h->plt.refcount inconsistency

[Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>]

Tristan: ping for the 2.28 branch.

Perhaps the commit-policy for the 2.28 branch is
free-for-all-maintainers now after the release, and I'm just
over-cautious?  (Yep, that's a leading question. :-)

brgds, H-P

> Date: Tue, 28 Mar 2017 23:48:19 +0200
> From: Hans-Peter Nilsson <hp@axis.com>

> I put the testcase-proper first in the patch, so I don't have to
> duplicate here the comment there regarding the issue.
> 
> As a courtesy to the very patient reporter, I'd like to put this on
> the 2.28-branch.  Ok?
> 
> diff --git a/ld/testsuite/ld-cris/pr16044.d b/ld/testsuite/ld-cris/pr16044.d
> new file mode 100644
> index 0000000..e5d373d
> --- /dev/null
> +++ b/ld/testsuite/ld-cris/pr16044.d
> @@ -0,0 +1,43 @@
> +#source: dso-4.s
> +#source: dso-2b.s
> +#source: dso-1c.s
> +#as: --pic --no-underscore --em=criself -I$srcdir/$subdir
> +#ld: --shared -m crislinux
> +#readelf: -s -r
> +
> +# PR 16044 is about a (compile-time-non-local) hidden function symbol,
> +# entered as an undef reference with a R_CRIS_32_PLT_GOTREL relocation
> +# referring to a hidden symbol, later defined.  Here, we invalidly
> +# incremented the h->plt.refcount (from -1) as part of that relocation
> +# processing.  There are some PLTGOT relocations.  As there are no
> +# circumstances requiring a PLT entry for this symbol, its PLT entry
> +# can be eliminated and the PLTGOT relocations can be made to a static
> +# element in the GOT, relocated with the absolute-to-relative
> +# R_CRIS_RELATIVE relocation without symbol lookup.  As part of
> +# eliminating unneeded PLT entries (and PLTGOT to "static" GOT
> +# elimination), a later pass noticed the inconsistency through an
> +# assert.
> +#
> +# The key points in this dump that may need future adjustments are the
> +# single dynamic relocation, that the dsofn symbol it points to, is
> +# local, its absence from the dynamic symbol table and that the
> +# relocation and symbol values match.
> +
> +Relocation section '\.rela\.dyn' at offset 0x[0-9a-f]+ contains 1 entries:
> + Offset[ 	]+Info[ 	]+Type[ 	]+Sym\.Value  Sym\. Name \+ Addend
> +[0-9a-f]+  0+[0-9a-f]+ R_CRIS_RELATIVE[ 	]+184
> +
> +Symbol table '\.dynsym' contains 7 entries:
> + +Num: +Value +Size +Type +Bind +Vis +Ndx +Name
> + +0: 0+ +0 +NOTYPE +LOCAL +DEFAULT +UND 
> + +1: [0-9a-f]+ +0 +SECTION +LOCAL +DEFAULT +5 
> + +2: [0-9a-f]+ +0 +FUNC +GLOBAL +DEFAULT +5 export_1
> + +3: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +7 __bss_start
> + +4: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +7 _edata
> + +5: [0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +7 _end
> + +6: [0-9a-f]+ +0 +FUNC +GLOBAL +DEFAULT +5 export_2
> +
> +Symbol table '\.symtab' contains [0-9]+ entries:
> +#...
> + +[0-9]+: 0+184  +2 FUNC + LOCAL + DEFAULT + 5 dsofn
> +#...
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 04d84f1..cf5e762 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,12 @@
> +2017-03-28  Hans-Peter Nilsson  <hp@axis.com>
> +
> +	PR ld/16044
> +	* elf32-cris.c (elf_cris_adjust_gotplt_to_got): Adjust BFD_ASSERT
> +	to handle a local symbol with a hash-symbol-entry; without PLT.
> +	Add BFD_ASSERT for an incidental case with GOT entry present.
> +	(cris_elf_check_relocs): Increment PLT refcount only if the symbol
> +	isn't forced-or-set local.
> +
>  2017-03-27  Pip Cet  <pipcet@gmail.com>
>  
>  	* wasm-module.c: New file to support WebAssembly modules.
> diff --git a/bfd/elf32-cris.c b/bfd/elf32-cris.c
> index 97b8cc3..d4bbceb 100644
> --- a/bfd/elf32-cris.c
> +++ b/bfd/elf32-cris.c
> @@ -2714,8 +2714,9 @@ elf_cris_adjust_gotplt_to_got (struct elf_cris_link_hash_entry *h, void * p)
>    struct bfd_link_info *info = (struct bfd_link_info *) p;
>  
>    /* A GOTPLT reloc, when activated, is supposed to be included into
> -     the PLT refcount.  */
> +     the PLT refcount, when the symbol isn't set-or-forced local.  */
>    BFD_ASSERT (h->gotplt_refcount == 0
> +	      || h->root.plt.refcount == -1
>  	      || h->gotplt_refcount <= h->root.plt.refcount);
>  
>    /* If nobody wanted a GOTPLT with this symbol, we're done.  */
> @@ -2741,6 +2742,7 @@ elf_cris_adjust_gotplt_to_got (struct elf_cris_link_hash_entry *h, void * p)
>        srelgot = elf_hash_table (info)->srelgot;
>  
>        /* Put accurate refcounts there.  */
> +      BFD_ASSERT (h->root.got.refcount >= 0);
>        h->root.got.refcount += h->gotplt_refcount;
>        h->reg_got_refcount = h->gotplt_refcount;
>  
> @@ -3476,7 +3478,10 @@ cris_elf_check_relocs (bfd *abfd,
>  	    continue;
>  
>  	  h->needs_plt = 1;
> -	  h->plt.refcount++;
> +
> +	  /* If the symbol is forced local, the refcount is unavailable.  */
> +	  if (h->plt.refcount != -1)
> +	    h->plt.refcount++;
>  	  break;
>  
>  	case R_CRIS_8:
> diff --git a/ld/ChangeLog b/ld/ChangeLog
> index d717cce..84ef78e 100644
> --- a/ld/ChangeLog
> +++ b/ld/ChangeLog
> @@ -1,3 +1,9 @@
> +2017-03-28  Hans-Peter Nilsson  <hp@axis.com>
> +
> +	PR ld/16044
> +	* testsuite/ld-cris/pr16044.d, testsuite/ld-cris/dso-1c.s,
> +	testsuite/ld-cris/dso-2b.s, testsuite/ld-cris/dso-4.s: New test.
> +
>  2017-03-21  Sandra Loosemore  <sandra@codesourcery.com>
>  
>  	* testsuite/lib/ld-lib.exp (check_shared_lib_support): Return
> diff --git a/ld/testsuite/ld-cris/dso-1c.s b/ld/testsuite/ld-cris/dso-1c.s
> new file mode 100644
> index 0000000..92ad4ee
> --- /dev/null
> +++ b/ld/testsuite/ld-cris/dso-1c.s
> @@ -0,0 +1,2 @@
> +	.include "dso-1.s"
> +	.hidden dsofn
> diff --git a/ld/testsuite/ld-cris/dso-2b.s b/ld/testsuite/ld-cris/dso-2b.s
> new file mode 100644
> index 0000000..f1fbf14
> --- /dev/null
> +++ b/ld/testsuite/ld-cris/dso-2b.s
> @@ -0,0 +1,2 @@
> +	.include "dso-2.s"
> +	.hidden dsofn
> diff --git a/ld/testsuite/ld-cris/dso-4.s b/ld/testsuite/ld-cris/dso-4.s
> new file mode 100644
> index 0000000..767a0d8
> --- /dev/null
> +++ b/ld/testsuite/ld-cris/dso-4.s
> @@ -0,0 +1,6 @@
> +	.text
> +	.global	export_2
> +	.type	export_2,@function
> +export_2:
> +	.hidden dsofn
> +	move.d dsofn:GOTOFF,$r4
>