public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org, "Fāng-ruì Sòng" <maskray@google.com>,
	"H.J. Lu" <hjl.tools@gmail.com>
Subject: Re: [PATCH 3/3] elf: Assume disjointed .rela.dyn and .rela.plt for loader
Date: Mon, 1 Nov 2021 13:44:03 -0300	[thread overview]
Message-ID: <6a6dee64-dd5e-e9d8-38fb-6e29165b466e@linaro.org> (raw)
In-Reply-To: <20211101125003.500945-4-adhemerval.zanella@linaro.org>



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
> 

  reply	other threads:[~2021-11-01 16:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-01 12:50 [PATCH 0/3] Fix lld build for armhf Adhemerval Zanella
2021-11-01 12:50 ` [PATCH 1/3] arm: Use internal symbol for _dl_argv on _dl_start_user Adhemerval Zanella
2021-11-01 16:38   ` Fāng-ruì Sòng
2021-11-01 12:50 ` [PATCH 2/3] arm: Use have-mtls-dialect-gnu2 to check for ARM TLS descriptors support Adhemerval Zanella
2021-11-01 16:42   ` Fāng-ruì Sòng
2021-11-01 12:50 ` [PATCH 3/3] elf: Assume disjointed .rela.dyn and .rela.plt for loader Adhemerval Zanella
2021-11-01 16:44   ` Adhemerval Zanella [this message]
2021-11-01 17:16     ` H.J. Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6a6dee64-dd5e-e9d8-38fb-6e29165b466e@linaro.org \
    --to=adhemerval.zanella@linaro.org \
    --cc=hjl.tools@gmail.com \
    --cc=libc-alpha@sourceware.org \
    --cc=maskray@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).