This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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: [PATCH] Fix ld.so regression.


On Tue, Apr 10, 2012 at 10:34 AM, David Miller <davem@davemloft.net> wrote:
> The _ELF_DYNAMIC_DO_RELOC simplifications went a bit too far.
> The one extra case we have to handle is when there is a GAP
> between the rel and the pltrel.
>
> The case where this can happen is extremely unfortunate, go
> look at:
>
> ? ? ? ?https://wiki.mozilla.org/Elfhack
>
> but please not if you've eaten recently.

Interesting. My opinion is that this is a reflection of how the
machines have chosen to implement relocations. Nothing says you can't
have one relocation process a whole batch of changes. In fact I
remember Power has a single relocation for a large array of OPDs that
need fixing up. Instead of this "crazy hacking" you could add a new
reloction that acts on a sorted batch of offsets.

> Anyways the following patch will fix it.
>
> I didn't plan on updating NEWS with the bug number, since this
> is fixing a bug introduced during development and not present
> in previous releases. ?Is that the right way to handle this?

No.

The consensus was that a simple policy is policy that gets followed.

For any bug fixed you put the BZ # into NEWS.

http://sourceware.org/ml/libc-alpha/2012-03/msg00299.html
http://sourceware.org/ml/libc-alpha/2012-02/msg00344.html

> Otherwise, ok to commit?
>
> 2012-04-10 ?David S. Miller ?<davem@davemloft.net>
>
> ? ? ? ?[BZ #13967]
> ? ? ? ?* elf/dynamic-link.h (_ELF_DYNAMIC_DO_RELOC): Handle the case
> ? ? ? ?where there is a gap between DT_REL(A) and DT_JMPREL.
>
> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index ef01c61..dc0a909 100644
> --- a/elf/dynamic-link.h
> +++ b/elf/dynamic-link.h
> @@ -252,9 +252,10 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
> ?/* On some machines, notably SPARC, DT_REL* includes DT_JMPREL in its
> ? ?range. ?Note that according to the ELF spec, this is completely legal!
>
> - ? We are guarenteed that we have one of two situations. ?Either DT_JMPREL
> + ? We are guarenteed that we have one of three situations. ?Either DT_JMPREL
> ? ?comes immediately after DT_REL*, or there is overlap and DT_JMPREL
> - ? consumes precisely the very end of the DT_REL*. ?*/
> + ? consumes precisely the very end of the DT_REL*, or DT_JMPREL and DT_REL*
> + ? are completely separate and there is a gap between them. ?*/
>
> ?# define _ELF_DYNAMIC_DO_RELOC(RELOC, reloc, map, do_lazy, skip_ifunc, test_rel) \
> ? do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> @@ -275,19 +276,20 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
> ? ? ? ?&& (!test_rel || (map)->l_info[DT_PLTREL]->d_un.d_val == DT_##RELOC)) \
> ? ? ? { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?ElfW(Addr) start = D_PTR ((map), l_info[DT_JMPREL]); ? ? ? ? ? ? ? ? ?\
> + ? ? ? ElfW(Addr) size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val; ? ? ? ? ? ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> - ? ? ? if (__builtin_expect (ranges[0].size, 1)) ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? if (ranges[0].start + ranges[0].size == (start + size)) ? ? ? ? ? ? ? \
> ? ? ? ? ?ranges[0].size = (start - ranges[0].start); ? ? ? ? ? ? ? ? ? ? ? ? \

Now that we have a `size', could you rewrite this to be less instructions?

e.g. ranges[0].size -= size;

> ? ? ? ?if (! ELF_DURING_STARTUP && ((do_lazy) || ranges[0].size == 0)) ? ? ? \
> ? ? ? ? ?{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ?ranges[1].start = start; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> - ? ? ? ? ? ranges[1].size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val; ? ? ? ? ?\
> + ? ? ? ? ? ranges[1].size = size; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ?ranges[1].lazy = (do_lazy); ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ?} ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ?else ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ?{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ? ?/* Combine processing the sections. ?*/ ? ? ? ? ? ? ? ? ? ? ? ? ? \
> - ? ? ? ? ? ranges[0].size += (map)->l_info[DT_PLTRELSZ]->d_un.d_val; ? ? ? ? \
> + ? ? ? ? ? ranges[0].size += size; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? ? ?} ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> ? ? ? } ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\

OK as is, and OK with the minor optimization.

Cheers,
Carlos.


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