This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: MIPS multigot fixes for Linux
- From: Richard Sandiford <rsandifo at redhat dot com>
- To: Daniel Jacobowitz <drow at mvista dot com>
- Cc: binutils at sources dot redhat dot com, Atsushi Nemoto <anemo at mba dot ocn dot ne dot jp>, Eric Christopher <echristo at redhat dot com>
- Date: 11 Nov 2003 23:43:39 +0000
- Subject: Re: MIPS multigot fixes for Linux
- References: <20031111173516.GA27967@nevyn.them.org>
> 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