elf backend hide_symbol bug

elf backend hide_symbol bug

[Alan Modra]

See http://sources.redhat.com/bugzilla/show_bug.cgi?id=293 for my
analysis of this problem.  mips, cris, xtensa maintainers should
note that there may be a similar problem for their ports.  The
backend hide_symbol functions can be called when the plt and got
unions are used as refcounts, eg. from bfd_elf_link_add_symbols,
and also when the field may be used as an offset/pointer, eg. from
_bfd_elf_adjust_dynamic_symbol.

	PR 293
	* elf32-hppa.c (elf32_hppa_hide_symbol): Use init_refcount to
	"zero" the plt field.

Index: bfd/elf32-hppa.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-hppa.c,v
retrieving revision 1.119
diff -u -p -r1.119 elf32-hppa.c
--- bfd/elf32-hppa.c	13 Aug 2004 03:15:57 -0000	1.119
+++ bfd/elf32-hppa.c	13 Aug 2004 08:14:37 -0000
@@ -1623,7 +1623,7 @@ elf32_hppa_hide_symbol (struct bfd_link_
   if (! ((struct elf32_hppa_link_hash_entry *) h)->plabel)
     {
       h->elf_link_hash_flags &= ~ELF_LINK_HASH_NEEDS_PLT;
-      h->plt.offset = (bfd_vma) -1;
+      h->plt = elf_hash_table (info)->init_refcount;
     }
 }
 

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: elf backend hide_symbol bug

[Bob Wilson]

Alan Modra wrote:
> See http://sources.redhat.com/bugzilla/show_bug.cgi?id=293 for my
> analysis of this problem.  mips, cris, xtensa maintainers should
> note that there may be a similar problem for their ports.  The
> backend hide_symbol functions can be called when the plt and got
> unions are used as refcounts, eg. from bfd_elf_link_add_symbols,
> and also when the field may be used as an offset/pointer, eg. from
> _bfd_elf_adjust_dynamic_symbol.
> 
> 	PR 293
> 	* elf32-hppa.c (elf32_hppa_hide_symbol): Use init_refcount to
> 	"zero" the plt field.
> 

I think the Xtensa port is OK -- it never uses the offset portion of the union 
and also ignores refcounts of -1.  I'm not completely certain, though, so I'll 
keep an eye out for problems in this area.  Thanks for the warning, Alan.

--Bob

Re: elf backend hide_symbol bug

[Hans-Peter Nilsson]

On Fri, 13 Aug 2004, Alan Modra wrote:
> See http://sources.redhat.com/bugzilla/show_bug.cgi?id=293 for my
> analysis of this problem.  mips, cris, xtensa maintainers should
> note that there may be a similar problem for their ports.

Thanks for the heads-up.  I need to investigate.  In any case, I
need to add a test-case to ld-cris for this.  I was thinking of
trying the description in comment #1.  Of course, it'd be
simpler to use an existing test-case that exposes a bug of this
type.  Did you perhaps write one for hppa for this bug?

brgds, H-P

Re: elf backend hide_symbol bug

[Alan Modra]

On Sun, Sep 05, 2004 at 11:18:44PM -0400, Hans-Peter Nilsson wrote:
> On Fri, 13 Aug 2004, Alan Modra wrote:
> > See http://sources.redhat.com/bugzilla/show_bug.cgi?id=293 for my
> > analysis of this problem.  mips, cris, xtensa maintainers should
> > note that there may be a similar problem for their ports.
> 
> Thanks for the heads-up.  I need to investigate.  In any case, I
> need to add a test-case to ld-cris for this.  I was thinking of
> trying the description in comment #1.  Of course, it'd be
> simpler to use an existing test-case that exposes a bug of this
> type.  Did you perhaps write one for hppa for this bug?

No, I just used the supplied .o files.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: elf backend hide_symbol bug

[Hans-Peter Nilsson]

On Fri, 13 Aug 2004, Alan Modra wrote:

> See http://sources.redhat.com/bugzilla/show_bug.cgi?id=293 for my
> analysis of this problem.  mips, cris, xtensa maintainers should
> note that there may be a similar problem for their ports.

I think I need a clue here...  Please bear with my denseness.

>  The
> backend hide_symbol functions can be called when the plt and got
> unions are used as refcounts, eg. from bfd_elf_link_add_symbols,
> and also when the field may be used as an offset/pointer, eg. from
> _bfd_elf_adjust_dynamic_symbol.

But elf32-cris.c doesn't.  It calls _bfd_elf_link_hash_hide_symbol
from its hide_symbol function which seems to DTRT.

The other function called from the hide_symbol function,
elf_cris_adjust_gotplt_to_got, could supposedly have some
similar problem for got refcounts, but I really need to
reproduce the suspicious case.  The critical piece is of course
to get the hide_symbol called when the plt and got unions are
transformed to be offsets rather than refcounts.

Can you please describe in terms of input what kind of objects
are needed to reproduce the bug exposure?

For example (insufficient; I'd need a GOT reference too, but I
can't even get the hide_symbol function called), I've tried to
do it by linking three object files -shared:

1: ld-cris/obj1.s (just a _start definition)
2: Like ld-cris/dso-1.s but with ".hidden dsofn".
3: An object that references "dsofn" with a PC-relative
   PLT-requesting reloc.

I'll try to find out for myself using the input files to PR293.

brgds, H-P

Re: elf backend hide_symbol bug

[Alan Modra]

On Mon, Sep 06, 2004 at 08:59:57PM -0400, Hans-Peter Nilsson wrote:
> On Fri, 13 Aug 2004, Alan Modra wrote:
> 
> > See http://sources.redhat.com/bugzilla/show_bug.cgi?id=293 for my
> > analysis of this problem.  mips, cris, xtensa maintainers should
> > note that there may be a similar problem for their ports.
> 
> I think I need a clue here...  Please bear with my denseness.

Please note the "may"!  I didn't go so far as to convince myself that
cris really does have a problem.  I just noted that in
elf_cris_adjust_gotplt_to_got, called from elf_cris_hide_symbol, you do

h->gotplt_refcount = -1

and in cris_elf_check_relocs, you

((struct elf_cris_link_hash_entry *) h)->gotplt_refcount++;

So, to tickle the bug on cris you need some combination of input files
that hides a symbol during the add_symbols phase of linking (visibility
or symbol versioning) and later references the same symbol using a
GOTPLT reloc.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Committed, CRIS: fix elf_cris_adjust_gotplt_to_got reinit (was: Re: elf backend hide_symbol bug)

[Hans-Peter Nilsson]

On Tue, 7 Sep 2004, Alan Modra wrote:
> I just noted that in
> elf_cris_adjust_gotplt_to_got, called from elf_cris_hide_symbol, you do
>
> h->gotplt_refcount = -1
>
> and in cris_elf_check_relocs, you
>
> ((struct elf_cris_link_hash_entry *) h)->gotplt_refcount++;

Got it.  Thanks Alan.

> So, to tickle the bug on cris you need some combination of input files
> that hides a symbol during the add_symbols phase of linking (visibility
> or symbol versioning) and later references the same symbol using a
> GOTPLT reloc.

Looking closer, the only *use* of h->gotplt_refcount (besides in
elf_cris_adjust_gotplt_to_got itself) is in
elf_cris_try_fold_plt_to_got, which is only called from
elf_cris_adjust_dynamic_symbol.  That call only happens if the
symbol was (also) defined in a DSO being linked against.
Because it is defined in a linked non-DSO object too, the old
(dynamic) definition is ignored in elflink.c:_bfd_elf_merge_symbol.

The -1 in "h->gotplt_refcount = -1" above should have been 0
(that's the initialization value), but the value doesn't
actually matter after this point.

What probably *can* go bad is through repeated calls to
elf_cris_adjust_gotplt_to_got, if the exactness of got reference
counts matters.  I'm still not sure.  I can't think of a
situation where relocs are removed, as --gc-sections is ignored
when -shared.  For an executable, I don't know at all.

I couldn't come up with a failing test-case, but I'm certainly
not sure of the safeness of the current (now previous)
situation.  The obvious correction to make repeated calls to
elf_cris_adjust_gotplt_to_got work correctly seemed TRT.

bfd:
	* elf32-cris.c (elf_cris_adjust_gotplt_to_got): Reset
	h->gotplt_refcount to 0, not -1.

Index: elf32-cris.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-cris.c,v
retrieving revision 1.54
diff -u -p -r1.54 elf32-cris.c
--- elf32-cris.c	7 Sep 2004 18:33:22 -0000	1.54
+++ elf32-cris.c	7 Sep 2004 22:33:17 -0000
@@ -1916,7 +1916,7 @@ elf_cris_adjust_gotplt_to_got (h, p)
 	 Probably not necessary at this stage, but keeping it accurate
 	 helps avoiding surprises later.  */
       h->root.got.refcount += h->gotplt_refcount;
-      h->gotplt_refcount = -1;
+      h->gotplt_refcount = 0;
     }
   else
     {
@@ -1928,7 +1928,7 @@ elf_cris_adjust_gotplt_to_got (h, p)
       /* Put an accurate refcount there.  */
       h->root.got.refcount = h->gotplt_refcount;

-      h->gotplt_refcount = -1;
+      h->gotplt_refcount = 0;

       /* We always have a .got and a .rela.got section if there were
 	 GOTPLT relocs in input.  */

brgds, H-P