This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix lazy setting for PLTREL overlap cpus.
- From: "Carlos O'Donell" <carlos at systemhalted dot org>
- To: David Miller <davem at davemloft dot net>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 3 Apr 2012 23:39:34 -0400
- Subject: Re: [PATCH] Fix lazy setting for PLTREL overlap cpus.
- References: <20120403.200249.837287680431052456.davem@davemloft.net>
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.