MIPS multigot fixes for Linux

[Richard Sandiford]

Daniel Jacobowitz <drow@mvista.com> writes:
> On Tue, Nov 11, 2003 at 11:43:39PM +0000, Richard Sandiford wrote:
> > It looks like the cases are:
> > 
> >                                    A    B    C    D    E
> >     ------------------------------------------------------
> >     symbol defined locally?        Y    Y    N    N    N
> >     symbol defined externally?     -    -    N    Y    -
> >     creating a DSO?                N    Y    N    N    Y
> >     ------------------------------------------------------
> >     relocation needed?             N    Y    N    Y    Y
> >     ------------------------------------------------------
> >     glibc relocation subtrahend:   -    0    -    0    0
> >     initial glibc entry:           S    0    S=0  S=0  S=0
> >     ------------------------------------------------------
> >     irix relocation subtrahend:    -    S    -    S=0  S=0
> >     initial irix entry:            S    S    S=0  S=0  S=0
> >     ------------------------------------------------------
> > 
> > where S = st_value.  Like you say, none of the symbols can be lazily
> > bound, so S should be 0 for all undefined symbols.  And it's really
> > just (B) that needs special handling.
> > 
> > Does that match your thinking?  If so, then your change to
> > _bfd_mips_elf_finish_dynamic_symbol looks good to me:
> 
> I'm not entirely sure what you mean for case C.

Undefined weak symbols.

> > Looking at the code above, I see VALUE is set by:
> > 
> >       if (info->shared
> > 	  || h->root.type == bfd_link_hash_undefined
> > 	  || h->root.type == bfd_link_hash_undefweak)
> > 	value = 0;
> >       else if (sym->st_value)
> > 	value = sym->st_value;
> >       else
> > 	value = h->root.u.def.value;
> > 
> > If the table above is right, then I think your patch will make this
> > equivalent to:
> > 
> >       value = st_value;
> > 
> > which IMO makes things easier to follow.
> 
> And later:
>  > Uh.  Equivalent for _glibc_.  For irix, it fixes a bug, since the
>  > relocation field should contain the symbol value even if info->shared.
> 
> I'm not sure about this.  I'd rather not make that change myself, since
> I don't understand it - for instance, I don't understand why
> h->root.u.def.value was there in the first place.  Did it have
> something to do with vestigial quickstart code?

Yeah, I think so.  h->root.u.def.value would be the value of the symbol
found in an external object.  And if we supported quickstart, that would
indeed be the right value to use, since it's also the value we'd put in
the primary GOT.

If we apply your patch, then VALUE is only inserted into the GOT if
SGI_COMPAT or if no relocation is needed.  And going by the table above,
I think the value we want is always st_value in this case.

> Besides, I don't see what you mean.  If the symbol has a value but we
> are emitting a relocation (for data objects, for instance) we must
> still put 0 in the GOT for glibc.

But doesn't your patch take care of that?  My point is that (from the
irix line above) the "right" thing to do is insert st_value for all
cases.  And your patch successfully works around the cases in glibc
where that isn't true.  Both changes together should do the right thing.

Richard