[3/3] elf: Assume disjointed .rela.dyn and .rela.plt for loader
Checks
Context |
Check |
Description |
dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
The linker might add another section between the two relocation section
for the loader as well. For instance on arm, lld does:
[ 7] .rel.dyn REL 000007f0 0007f0 000088 08 A 1 0 4
[ 8] .ARM.exidx ARM_EXIDX 00000878 000878 0000b0 00 AL1 2 0 4
[ 9] .rel.plt REL 00000928 000928 000028 08 AI 1 17 4
This patch removes the ELF_DURING_STARTUP optimization and assume
both section might no be linear.
Checked on x86_64, i686, aarch64, armhf, powerpc64le, powerpc64,
and powerpc (to check if this breaks on different architectures).
---
elf/dynamic-link.h | 32 +++++++++-----------------------
1 file changed, 9 insertions(+), 23 deletions(-)
Comments
On 01/11/2021 09:50, Adhemerval Zanella wrote:
> The linker might add another section between the two relocation section
> for the loader as well. For instance on arm, lld does:
>
> [ 7] .rel.dyn REL 000007f0 0007f0 000088 08 A 1 0 4
> [ 8] .ARM.exidx ARM_EXIDX 00000878 000878 0000b0 00 AL1 2 0 4
> [ 9] .rel.plt REL 00000928 000928 000028 08 AI 1 17 4
>
> This patch removes the ELF_DURING_STARTUP optimization and assume
> both section might no be linear.
>
> Checked on x86_64, i686, aarch64, armhf, powerpc64le, powerpc64,
> and powerpc (to check if this breaks on different architectures).
Following up the discussion on glibc call, below it an update of the
patch description.
---
The patch removes the the ELF_DURING_STARTUP optimization and assume
both .rel.dyn and .rel.plt might no be linear. This allows some
code simplification since relocation will be handled independently
where it done on bootstrap.
I could not see any performance implications, at least on x86_64.
Running 10000 time the command
LD_DEBUG=statistics ./elf/ld.so ./libc.so
And filtering the "total startup time in dynamic loader" result,
the geometric mean I see are:
patched master
Ryzen 7 5900x 24140 24952
i7-4510U 45957 45982
(the results do show some variation, I did not make any statistical
analysis).
It also allows build arm with lld, since it inserts ".ARM.exidx"
between ".rel.dyn" and ".rel.plt" for the loader.
Checked on x86_64-linux-gnu and arm-linux-gnueabihf.
> ---
> elf/dynamic-link.h | 32 +++++++++-----------------------
> 1 file changed, 9 insertions(+), 23 deletions(-)
>
> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index ac4cc70dea..f619615e5c 100644
> --- a/elf/dynamic-link.h
> +++ b/elf/dynamic-link.h
> @@ -65,12 +65,6 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>
> #ifdef RESOLVE_MAP
>
> -# if defined RTLD_BOOTSTRAP || defined STATIC_PIE_BOOTSTRAP
> -# define ELF_DURING_STARTUP (1)
> -# else
> -# define ELF_DURING_STARTUP (0)
> -# endif
> -
> /* Get the definitions of `elf_dynamic_do_rel' and `elf_dynamic_do_rela'.
> These functions are almost identical, so we use cpp magic to avoid
> duplicating their code. It cannot be done in a more general function
> @@ -106,9 +100,8 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> \
> if (ranges[0].start + ranges[0].size == (start + size)) \
> ranges[0].size -= size; \
> - if (ELF_DURING_STARTUP \
> - || (!(do_lazy) \
> - && (ranges[0].start + ranges[0].size) == start)) \
> + if (!(do_lazy) \
> + && (ranges[0].start + ranges[0].size) == start) \
> { \
> /* Combine processing the sections. */ \
> ranges[0].size += size; \
> @@ -121,20 +114,13 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> } \
> } \
> \
> - if (ELF_DURING_STARTUP) \
> - elf_dynamic_do_##reloc ((map), scope, ranges[0].start, ranges[0].size, \
> - ranges[0].nrelative, 0, skip_ifunc); \
> - else \
> - { \
> - int ranges_index; \
> - for (ranges_index = 0; ranges_index < 2; ++ranges_index) \
> - elf_dynamic_do_##reloc ((map), scope, \
> - ranges[ranges_index].start, \
> - ranges[ranges_index].size, \
> - ranges[ranges_index].nrelative, \
> - ranges[ranges_index].lazy, \
> - skip_ifunc); \
> - } \
> + for (int ranges_index = 0; ranges_index < 2; ++ranges_index) \
> + elf_dynamic_do_##reloc ((map), scope, \
> + ranges[ranges_index].start, \
> + ranges[ranges_index].size, \
> + ranges[ranges_index].nrelative, \
> + ranges[ranges_index].lazy, \
> + skip_ifunc); \
> } while (0)
>
> # if ELF_MACHINE_NO_REL || ELF_MACHINE_NO_RELA
>
On Mon, Nov 1, 2021 at 9:44 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 01/11/2021 09:50, Adhemerval Zanella wrote:
> > The linker might add another section between the two relocation section
> > for the loader as well. For instance on arm, lld does:
> >
> > [ 7] .rel.dyn REL 000007f0 0007f0 000088 08 A 1 0 4
> > [ 8] .ARM.exidx ARM_EXIDX 00000878 000878 0000b0 00 AL1 2 0 4
> > [ 9] .rel.plt REL 00000928 000928 000028 08 AI 1 17 4
> >
> > This patch removes the ELF_DURING_STARTUP optimization and assume
> > both section might no be linear.
> >
> > Checked on x86_64, i686, aarch64, armhf, powerpc64le, powerpc64,
> > and powerpc (to check if this breaks on different architectures).
>
> Following up the discussion on glibc call, below it an update of the
> patch description.
>
> ---
>
> The patch removes the the ELF_DURING_STARTUP optimization and assume
> both .rel.dyn and .rel.plt might no be linear. This allows some
> code simplification since relocation will be handled independently
> where it done on bootstrap.
>
> I could not see any performance implications, at least on x86_64.
> Running 10000 time the command
>
> LD_DEBUG=statistics ./elf/ld.so ./libc.so
>
> And filtering the "total startup time in dynamic loader" result,
> the geometric mean I see are:
>
> patched master
> Ryzen 7 5900x 24140 24952
> i7-4510U 45957 45982
>
> (the results do show some variation, I did not make any statistical
> analysis).
>
> It also allows build arm with lld, since it inserts ".ARM.exidx"
> between ".rel.dyn" and ".rel.plt" for the loader.
>
> Checked on x86_64-linux-gnu and arm-linux-gnueabihf.
>
> > ---
> > elf/dynamic-link.h | 32 +++++++++-----------------------
> > 1 file changed, 9 insertions(+), 23 deletions(-)
> >
> > diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> > index ac4cc70dea..f619615e5c 100644
> > --- a/elf/dynamic-link.h
> > +++ b/elf/dynamic-link.h
> > @@ -65,12 +65,6 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> >
> > #ifdef RESOLVE_MAP
> >
> > -# if defined RTLD_BOOTSTRAP || defined STATIC_PIE_BOOTSTRAP
> > -# define ELF_DURING_STARTUP (1)
> > -# else
> > -# define ELF_DURING_STARTUP (0)
> > -# endif
> > -
> > /* Get the definitions of `elf_dynamic_do_rel' and `elf_dynamic_do_rela'.
> > These functions are almost identical, so we use cpp magic to avoid
> > duplicating their code. It cannot be done in a more general function
> > @@ -106,9 +100,8 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> > \
> > if (ranges[0].start + ranges[0].size == (start + size)) \
> > ranges[0].size -= size; \
> > - if (ELF_DURING_STARTUP \
> > - || (!(do_lazy) \
> > - && (ranges[0].start + ranges[0].size) == start)) \
> > + if (!(do_lazy) \
> > + && (ranges[0].start + ranges[0].size) == start) \
> > { \
> > /* Combine processing the sections. */ \
> > ranges[0].size += size; \
> > @@ -121,20 +114,13 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> > } \
> > } \
> > \
> > - if (ELF_DURING_STARTUP) \
> > - elf_dynamic_do_##reloc ((map), scope, ranges[0].start, ranges[0].size, \
> > - ranges[0].nrelative, 0, skip_ifunc); \
> > - else \
> > - { \
> > - int ranges_index; \
> > - for (ranges_index = 0; ranges_index < 2; ++ranges_index) \
> > - elf_dynamic_do_##reloc ((map), scope, \
> > - ranges[ranges_index].start, \
> > - ranges[ranges_index].size, \
> > - ranges[ranges_index].nrelative, \
> > - ranges[ranges_index].lazy, \
> > - skip_ifunc); \
> > - } \
> > + for (int ranges_index = 0; ranges_index < 2; ++ranges_index) \
> > + elf_dynamic_do_##reloc ((map), scope, \
> > + ranges[ranges_index].start, \
> > + ranges[ranges_index].size, \
> > + ranges[ranges_index].nrelative, \
> > + ranges[ranges_index].lazy, \
> > + skip_ifunc); \
> > } while (0)
> >
> > # if ELF_MACHINE_NO_REL || ELF_MACHINE_NO_RELA
> >
Please submit the v2 patch with the updated commit log.
Thanks.
@@ -65,12 +65,6 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
#ifdef RESOLVE_MAP
-# if defined RTLD_BOOTSTRAP || defined STATIC_PIE_BOOTSTRAP
-# define ELF_DURING_STARTUP (1)
-# else
-# define ELF_DURING_STARTUP (0)
-# endif
-
/* Get the definitions of `elf_dynamic_do_rel' and `elf_dynamic_do_rela'.
These functions are almost identical, so we use cpp magic to avoid
duplicating their code. It cannot be done in a more general function
@@ -106,9 +100,8 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
\
if (ranges[0].start + ranges[0].size == (start + size)) \
ranges[0].size -= size; \
- if (ELF_DURING_STARTUP \
- || (!(do_lazy) \
- && (ranges[0].start + ranges[0].size) == start)) \
+ if (!(do_lazy) \
+ && (ranges[0].start + ranges[0].size) == start) \
{ \
/* Combine processing the sections. */ \
ranges[0].size += size; \
@@ -121,20 +114,13 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
} \
} \
\
- if (ELF_DURING_STARTUP) \
- elf_dynamic_do_##reloc ((map), scope, ranges[0].start, ranges[0].size, \
- ranges[0].nrelative, 0, skip_ifunc); \
- else \
- { \
- int ranges_index; \
- for (ranges_index = 0; ranges_index < 2; ++ranges_index) \
- elf_dynamic_do_##reloc ((map), scope, \
- ranges[ranges_index].start, \
- ranges[ranges_index].size, \
- ranges[ranges_index].nrelative, \
- ranges[ranges_index].lazy, \
- skip_ifunc); \
- } \
+ for (int ranges_index = 0; ranges_index < 2; ++ranges_index) \
+ elf_dynamic_do_##reloc ((map), scope, \
+ ranges[ranges_index].start, \
+ ranges[ranges_index].size, \
+ ranges[ranges_index].nrelative, \
+ ranges[ranges_index].lazy, \
+ skip_ifunc); \
} while (0)
# if ELF_MACHINE_NO_REL || ELF_MACHINE_NO_RELA