[PATCH] Fix lazy setting for PLTREL overlap cpus.

[Carlos O'Donell]

On Tue, Apr 3, 2012 at 8:02 PM, David Miller <davem@davemloft.net> wrote:
> While investigating a problem with unused dep detection on sparc, I
> found that several bug fixes and improvements have been going into one
> of the two _ELF_DYNAMIC_DO_RELOC definitions and not the other.
>
> I'll slowly try to bring the in line with eachother again, and
> here's the first step.
>
> Any objections?

Yes. I don't think the behaviour of the change matches the behaviour
of the non-overlap case.

> 2012-04-03  David S. Miller  <davem@davemloft.net>
>
>        * elf/dynamic-link.h (_ELF_DYNAMIC_DO_RELOC): Set lazy properly in
>        the PLTREL overlap definition just like the non-PLTREL overlap one.
>
> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index aa71227..669180e 100644
> --- a/elf/dynamic-link.h
> +++ b/elf/dynamic-link.h
> @@ -263,7 +263,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
>     int ranges_index;                                                        \
>                                                                              \
>     ranges[0].lazy = ranges[2].lazy = 0;                                     \
> -    ranges[1].lazy = 1;                                                              \
> +    ranges[1].lazy = (do_lazy);                                                      \
>     ranges[0].size = ranges[1].size = ranges[2].size = 0;                    \
>     ranges[0].nrelative = ranges[1].nrelative = ranges[2].nrelative = 0;      \
>                                                                              \

If you'r going to do an incremental cleanup, this might be better?
~~~
diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
index aa71227..09ddd0d 100644
--- a/elf/dynamic-link.h
+++ b/elf/dynamic-link.h
@@ -262,8 +262,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
     ranges[3];                                                               \
     int ranges_index;                                                        \
                                                                              \
-    ranges[0].lazy = ranges[2].lazy = 0;                                     \
-    ranges[1].lazy = 1;
               \
+    ranges[0].lazy = ranges[1].lazy = ranges[2].lazy = 0;                    \
     ranges[0].size = ranges[1].size = ranges[2].size = 0;                    \
     ranges[0].nrelative = ranges[1].nrelative = ranges[2].nrelative = 0;      \
                                                                              \
@@ -283,6 +282,7 @@ elf_get_dynamic_info (struct link_map *l, ElfW(Dyn) *temp)
       {
               \
        ranges[1].start = D_PTR ((map), l_info[DT_JMPREL]);                   \
        ranges[1].size = (map)->l_info[DT_PLTRELSZ]->d_un.d_val;              \
+       ranges[1].lazy = (do_lazy);                                           \
        ranges[2].start = ranges[1].start + ranges[1].size;                   \
        ranges[2].size = ranges[0].start + ranges[0].size - ranges[2].start;  \
        ranges[0].size = ranges[1].start - ranges[0].start;                   \
~~~
e.g. Only assign to ranges[1].lazy if you have a PLT for which you
might care about doing lazy reloc processing for.

It's unfortunate that the code isn't close to identical, how *optimal*
does this really need to be?

We need some ASCII art here:

Overlap case:
(a) Start with:
|<---DT_REL(A)------------------------------>|
              |<---DT_JMPREL--->|
(b) Compute
|<---- r0 --->|<----- r1 ------>|<--- r2 --->|

vs.

Non-overlap case:
|<---DT_REL ---->|<---DT_JMPREL--->|
|<--- r0 ------->|<--- r1 -------->|

This macro could do with some serious cleanup and commenting. I know
what it does only because I've stared long and hard at it over the
years (and I've read the ELF pdf back to front a couple of times).
That doesn't mean that others have any clue what's going on here
though and hints would be nice.

Note that this effects s390, sparc32/64, power32, and tile since they
all define ELF_MACHINE_PLTREL_OVERLAP. We might want to poke those
maintainers to test your patch?

Cheers,
Carlos.