[PATCH v2 1/2] elf: Introduce is_rtld_link_map

Adhemerval Zanella Netto adhemerval.zanella@linaro.org
Fri Dec 20 14:15:27 GMT 2024



On 17/12/24 09:51, Florian Weimer wrote:
> Unconditionally define it to false for static builds.
> 
> This avoids the awkward use of weak_extern for _dl_rtld_map
> in checks that cannot be possibly true on static builds.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
> v2: Remove dependency on second patch.
>  elf/dl-dst.h                           |  7 +------
>  elf/do-rel.h                           | 11 +----------
>  elf/dynamic-link.h                     |  2 +-
>  elf/rtld.c                             |  2 +-
>  sysdeps/arm/dl-machine.h               | 11 +----------
>  sysdeps/generic/ldsodefs.h             | 16 +++++++++++++++-
>  sysdeps/mips/dl-machine.h              | 16 +++-------------
>  sysdeps/powerpc/powerpc64/dl-machine.h |  4 ++--
>  sysdeps/sh/dl-machine.h                | 13 ++-----------
>  sysdeps/x86/dl-prop.h                  |  2 +-
>  10 files changed, 28 insertions(+), 56 deletions(-)
> 
> diff --git a/elf/dl-dst.h b/elf/dl-dst.h
> index 07100e5ff9..bff3685b83 100644
> --- a/elf/dl-dst.h
> +++ b/elf/dl-dst.h
> @@ -18,11 +18,6 @@
>  
>  #include "trusted-dirs.h"
>  
> -#ifdef SHARED
> -# define IS_RTLD(l) (l) == &GL(dl_rtld_map)
> -#else
> -# define IS_RTLD(l) 0
> -#endif
>  /* Guess from the number of DSTs the length of the result string.  */
>  #define DL_DST_REQUIRED(l, name, len, cnt) \
>    ({									      \
> @@ -44,7 +39,7 @@
>  	   auditing, in ld.so.  */					      \
>  	if ((l)->l_origin == NULL)					      \
>  	  {								      \
> -	    assert ((l)->l_name[0] == '\0' || IS_RTLD (l));		      \
> +	    assert ((l)->l_name[0] == '\0' || is_rtld_link_map (l));	      \
>  	    (l)->l_origin = _dl_get_origin ();				      \
>  	    dst_len = ((l)->l_origin && (l)->l_origin != (char *) -1	      \
>  			  ? strlen ((l)->l_origin) : 0);		      \
> diff --git a/elf/do-rel.h b/elf/do-rel.h
> index afea61cfd2..f9841966d5 100644
> --- a/elf/do-rel.h
> +++ b/elf/do-rel.h
> @@ -102,16 +102,7 @@ elf_dynamic_do_Rel (struct link_map *map, struct r_scope_elem *scope[],
>    else
>  #endif
>      {
> -      /* This is defined in rtld.c, but nowhere in the static libc.a; make
> -	 the reference weak so static programs can still link.  This
> -	 declaration cannot be done when compiling rtld.c (i.e. #ifdef
> -	 RTLD_BOOTSTRAP) because rtld.c contains the common defn for
> -	 _dl_rtld_map, which is incompatible with a weak decl in the same
> -	 file.  */
> -# ifndef SHARED
> -      weak_extern (GL(dl_rtld_map));
> -# endif
> -      if (map != &GL(dl_rtld_map)) /* Already done in rtld itself.  */
> +      if (!is_rtld_link_map (map)) /* Already done in rtld itself.  */
>  # if !defined DO_RELA || defined ELF_MACHINE_REL_RELATIVE
>  	/* Rela platforms get the offset from r_addend and this must
>  	   be copied in the relocation address.  Therefore we can skip
> diff --git a/elf/dynamic-link.h b/elf/dynamic-link.h
> index 370b0e872e..7649ea89e5 100644
> --- a/elf/dynamic-link.h
> +++ b/elf/dynamic-link.h
> @@ -192,7 +192,7 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>    do {									      \
>      int edr_lazy = elf_machine_runtime_setup ((map), (scope), (lazy),	      \
>  					      (consider_profile));	      \
> -    if (((map) != &GL(dl_rtld_map) || DO_RTLD_BOOTSTRAP))		      \
> +    if (!is_rtld_link_map (map) || DO_RTLD_BOOTSTRAP)			      \
>        ELF_DYNAMIC_DO_RELR (map);					      \
>      ELF_DYNAMIC_DO_REL ((map), (scope), edr_lazy, skip_ifunc);		      \
>      ELF_DYNAMIC_DO_RELA ((map), (scope), edr_lazy, skip_ifunc);		      \
> diff --git a/elf/rtld.c b/elf/rtld.c
> index b8cc3f605f..44bcf29ff6 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1960,7 +1960,7 @@ dl_main (const ElfW(Phdr) *phdr,
>      GL(dl_rtld_map).l_next->l_prev = GL(dl_rtld_map).l_prev;
>  
>    for (i = 1; i < main_map->l_searchlist.r_nlist; ++i)
> -    if (main_map->l_searchlist.r_list[i] == &GL(dl_rtld_map))
> +    if (is_rtld_link_map (main_map->l_searchlist.r_list[i]))
>        break;
>  
>    /* Insert the link map for the dynamic loader into the chain in
> diff --git a/sysdeps/arm/dl-machine.h b/sysdeps/arm/dl-machine.h
> index 9186831be3..b328287a18 100644
> --- a/sysdeps/arm/dl-machine.h
> +++ b/sysdeps/arm/dl-machine.h
> @@ -351,16 +351,7 @@ elf_machine_rel (struct link_map *map, struct r_scope_elem *scope[],
>  	  {
>  	    ElfW(Addr) tmp;
>  # ifndef RTLD_BOOTSTRAP
> -	   /* This is defined in rtld.c, but nowhere in the static
> -	      libc.a; make the reference weak so static programs can
> -	      still link.  This declaration cannot be done when
> -	      compiling rtld.c (i.e.  #ifdef RTLD_BOOTSTRAP) because
> -	      rtld.c contains the common defn for _dl_rtld_map, which
> -	      is incompatible with a weak decl in the same file.  */
> -#  ifndef SHARED
> -	    weak_extern (_dl_rtld_map);
> -#  endif
> -	    if (map == &GL(dl_rtld_map))
> +	    if (is_rtld_link_map (map))
>  	      /* Undo the relocation done here during bootstrapping.
>  		 Now we will relocate it anew, possibly using a
>  		 binding found in the user program or a loaded library
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 91447a5e77..84724ac37b 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1329,10 +1329,17 @@ rtld_active (void)
>    return GLRO(dl_init_all_dirs) != NULL;
>  }
>  
> +/* Returns true of L is the link map of the dynamic linker itself.  */
> +static inline bool
> +is_rtld_link_map (const struct link_map *l)
> +{
> +  return l == &GL(dl_rtld_map);
> +}
> +
>  static inline struct auditstate *
>  link_map_audit_state (struct link_map *l, size_t index)
>  {
> -  if (l == &GL (dl_rtld_map))
> +  if (is_rtld_link_map (l))
>      /* The auditstate array is stored separately.  */
>      return &GL (dl_rtld_auditstate) [index];
>    else
> @@ -1395,6 +1402,13 @@ void DL_ARCH_FIXUP_ATTRIBUTE _dl_audit_pltexit (struct link_map *l,
>    attribute_hidden;
>  
>  #else  /* !SHARED */
> +/* No special dynamic linker link map in static builds.  */
> +static inline bool
> +is_rtld_link_map (const struct link_map *l)
> +{
> +  return false;
> +}
> +
>  static inline void
>  _dl_audit_objclose (struct link_map *l)
>  {
> diff --git a/sysdeps/mips/dl-machine.h b/sysdeps/mips/dl-machine.h
> index 10e30f1e90..7a361edd4d 100644
> --- a/sysdeps/mips/dl-machine.h
> +++ b/sysdeps/mips/dl-machine.h
> @@ -436,16 +436,6 @@ elf_machine_reloc (struct link_map *map, struct r_scope_elem *scope[],
>    const unsigned long int r_type = ELFW(R_TYPE) (r_info);
>    ElfW(Addr) *addr_field = (ElfW(Addr) *) reloc_addr;
>  
> -#if !defined RTLD_BOOTSTRAP && !defined SHARED
> -  /* This is defined in rtld.c, but nowhere in the static libc.a;
> -     make the reference weak so static programs can still link.  This
> -     declaration cannot be done when compiling rtld.c (i.e.  #ifdef
> -     RTLD_BOOTSTRAP) because rtld.c contains the common defn for
> -     _dl_rtld_map, which is incompatible with a weak decl in the same
> -     file.  */
> -  weak_extern (GL(dl_rtld_map));
> -#endif
> -
>    switch (r_type)
>      {
>  #if !defined (RTLD_BOOTSTRAP)
> @@ -534,7 +524,7 @@ elf_machine_reloc (struct link_map *map, struct r_scope_elem *scope[],
>  		   though it's not ABI compliant.  Some day we should
>  		   bite the bullet and stop doing this.  */
>  #ifndef RTLD_BOOTSTRAP
> -		if (map != &GL(dl_rtld_map))
> +		if (!is_rtld_link_map (map))
>  #endif
>  		  reloc_value += SYMBOL_ADDRESS (map, sym, true);
>  	      }
> @@ -553,7 +543,7 @@ elf_machine_reloc (struct link_map *map, struct r_scope_elem *scope[],
>  	  }
>  	else
>  #ifndef RTLD_BOOTSTRAP
> -	  if (map != &GL(dl_rtld_map))
> +	  if (!is_rtld_link_map (map))
>  #endif
>  	    reloc_value += map->l_addr;
>  
> @@ -749,7 +739,7 @@ elf_machine_got_rel (struct link_map *map, struct r_scope_elem *scope[], int laz
>  
>    n = map->l_info[DT_MIPS (LOCAL_GOTNO)]->d_un.d_val;
>    /* The dynamic linker's local got entries have already been relocated.  */
> -  if (map != &GL(dl_rtld_map))
> +  if (!is_rtld_link_map (map))
>      {
>        /* got[0] is reserved. got[1] is also reserved for the dynamic object
>  	 generated by gnu ld. Skip these reserved entries from relocation.  */
> diff --git a/sysdeps/powerpc/powerpc64/dl-machine.h b/sysdeps/powerpc/powerpc64/dl-machine.h
> index 2b6f5d2b08..5855393b2b 100644
> --- a/sysdeps/powerpc/powerpc64/dl-machine.h
> +++ b/sysdeps/powerpc/powerpc64/dl-machine.h
> @@ -537,7 +537,7 @@ elf_machine_fixup_plt (struct link_map *map, lookup_t sym_map,
>    if (finaladdr != 0 && map != sym_map && !sym_map->l_relocated
>  #if !defined RTLD_BOOTSTRAP && defined SHARED
>        /* Bootstrap map doesn't have l_relocated set for it.  */
> -      && sym_map != &GL(dl_rtld_map)
> +      && !is_rtld_link_map (sym_map)
>  #endif
>        )
>      offset = sym_map->l_addr;
> @@ -662,7 +662,7 @@ resolve_ifunc (Elf64_Addr value,
>    if (map != sym_map
>  # if !defined RTLD_BOOTSTRAP && defined SHARED
>        /* Bootstrap map doesn't have l_relocated set for it.  */
> -      && sym_map != &GL(dl_rtld_map)
> +      && !is_rtld_link_map (map)
>  # endif
>        && !sym_map->l_relocated)
>      {
> diff --git a/sysdeps/sh/dl-machine.h b/sysdeps/sh/dl-machine.h
> index c2c970d0c4..01cd5114c4 100644
> --- a/sysdeps/sh/dl-machine.h
> +++ b/sysdeps/sh/dl-machine.h
> @@ -284,7 +284,7 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>    if (__glibc_unlikely (r_type == R_SH_RELATIVE))
>      {
>  #ifndef RTLD_BOOTSTRAP
> -      if (map != &GL(dl_rtld_map)) /* Already done in rtld itself.	 */
> +      if (is_rtld_link_map (map)) /* Already done in rtld itself.  */
>  #endif
>  	{
>  	  if (reloc->r_addend)
> @@ -380,16 +380,7 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>  	case R_SH_DIR32:
>  	  {
>  #if !defined RTLD_BOOTSTRAP
> -	   /* This is defined in rtld.c, but nowhere in the static
> -	      libc.a; make the reference weak so static programs can
> -	      still link.  This declaration cannot be done when
> -	      compiling rtld.c (i.e. #ifdef RTLD_BOOTSTRAP) because
> -	      rtld.c contains the common defn for _dl_rtld_map, which
> -	      is incompatible with a weak decl in the same file.  */
> -# ifndef SHARED
> -	    weak_extern (_dl_rtld_map);
> -# endif
> -	    if (map == &GL(dl_rtld_map))
> +	    if (is_rtld_link_map (map))
>  	      /* Undo the relocation done here during bootstrapping.
>  		 Now we will relocate it anew, possibly using a
>  		 binding found in the user program or a loaded library
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 08387dfaff..62619d7dbe 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -46,7 +46,7 @@ dl_isa_level_check (struct link_map *m, const char *program)
>  #ifdef SHARED
>        /* Skip ISA level check for ld.so since ld.so won't run if its ISA
>  	 level is higher than CPU.  */
> -      if (l == &GL(dl_rtld_map) || l->l_real == &GL(dl_rtld_map))
> +      if (is_rtld_link_map (l) || is_rtld_link_map (l->l_real))
>  	continue;
>  #endif
>  
> 
> base-commit: 87cd94bba4091d22e24116298ade33b712ada235
> prerequisite-patch-id: c9ba33bbc4825db275743eb86611ff8f09629acc
> prerequisite-patch-id: b7d52ec947792319fae789c7167a5b53f5cecc07
> prerequisite-patch-id: 58340abac5aa1f27f2049ac48e09fc50277bce18



More information about the Libc-alpha mailing list