This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: MIPS multigot fixes for Linux


> I humbly request an opinion from Richard S, who understands this stuff a
> whole lot better than I do.

Huh.  You give me too much credit, but I'll have a go. ;)

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:

> -	      if ((info->shared
> -		   || (elf_hash_table (info)->dynamic_sections_created
> -		       && p->d.h != NULL
> -		       && ((p->d.h->root.elf_link_hash_flags
> -			    & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
> -		       && ((p->d.h->root.elf_link_hash_flags
> -			    & ELF_LINK_HASH_DEF_REGULAR) == 0)))
> -		  && ! (mips_elf_create_dynamic_relocation
> -			(output_bfd, info, rel,
> -			 e.d.h, NULL, value, &addend, sgot)))
> -		return FALSE;
> -	      BFD_ASSERT (addend == 0);
> +	      if (info->shared
> +		  || (elf_hash_table (info)->dynamic_sections_created
> +		      && p->d.h != NULL
> +		      && ((p->d.h->root.elf_link_hash_flags
> +			   & ELF_LINK_HASH_DEF_DYNAMIC) != 0)
> +		      && ((p->d.h->root.elf_link_hash_flags
> +			   & ELF_LINK_HASH_DEF_REGULAR) == 0)))
> +		{
> +		  /* Unfortunately this is another one of the differences
> +		     between glibc and the Irix rld.  */
> +		  if (!SGI_COMPAT (output_bfd))
> +		    MIPS_ELF_PUT_WORD (output_bfd, 0, sgot->contents + offset);
> +
> +		  if (! (mips_elf_create_dynamic_relocation
> +			 (output_bfd, info, rel,
> +			  e.d.h, NULL, value, &addend, sgot)))
> +		    return FALSE;
> +		  BFD_ASSERT (addend == 0);
> +		}

I guess a more verbose comment might be useful though.  How about changing
it to something like:

      /* ??? The dynamic linker should subtract the original value of the
         symbol's GOT entry (which is always st_value) and add in the final
         value.  However, glibc's ld.so just adds the final value, so the
         in-place addend must be zero.  */

Also, as a very minor clean-up, you could bring "addend" into the new block.
It shouldn't need to be initialised to zero then.

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.

I'll try to look at the no_fn_stub thing tomorrow.

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]