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


Daniel Jacobowitz <drow@mvista.com> writes:
> 	* elfxx-mips.c (mips_elf_set_global_got_offset): Don't set
> 	no_fn_stub.
> 	(mips_elf_set_no_stub): New function.
> 	(mips_elf_multi_got): Call it.
> 	(_bfd_mips_elf_finish_dynamic_symbol): Fill relocated GOT entries
> 	with zero for ! SGI_COMPAT.  Simplify calculation of value.

Looks good to me FWIW.  One thing though:

> +      /* We can't do lazy update of GOT entries for non-primary GOTs since
> +        the PLT entries don't use the right offsets, so punt at it for now.
> +        We set this here because we are called via mips_elf_multi_got
> +        before _bfd_mips_elf_adjust_dynamic_symbol reads the no_fn_stub
> +        flag; this only matters for the global case, but
> +        _bfd_mips_elf_size_dynamic_sections is too late.  */
> +      entry->d.h->no_fn_stub = TRUE;

I'm not sure whether it's useful to keep this comment, especially given
the new ones you've added above the function itself, and above the call
in mips_elf_multi_got.  "Punt at it for now" makes it sound like a FIXME.
But surely it's just Plain Wrong to allow lazy binding of functions that
are pointed to by a secondary GOT?  From the dynamic linker's point of
view, it's no different from a function pointer in .data or whatever.

And setting no_fn_stub in its new position seems natural enough to me.
I'm not sure you need to excuse it.  If you do still want to mention
_bfd_mips_elf_size_dynamic_sections, it might be better to do it above
the call in mips_elf_multi_got.  But IMO it could be dropped altogether.

Richard


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