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 lazy setting for PLTREL overlap cpus.


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.


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