diff mbox series

elf: Remove unused l_text_end field from struct link_map

Message ID 87il8k7vxo.fsf@oldenburg.str.redhat.com
State New
Headers show
Series elf: Remove unused l_text_end field from struct link_map | expand

Commit Message

Florian Weimer Sept. 8, 2023, 11:20 a.m. UTC
It is a left-over from commit 52a01100ad011293197637e42b5be1a479a2
("elf: Remove ad-hoc restrictions on dlopen callers [BZ #22787]").

When backporting commmit 6985865bc3ad5b23147ee73466583dd7fdf65892
("elf: Always call destructors in reverse constructor order
(bug 30785)"), we can move the l_init_called_next field to this
place, so that the internal GLIBC_PRIVATE ABI does not change.

---
 elf/dl-load.c    | 2 +-
 elf/dl-load.h    | 7 ++-----
 elf/rtld.c       | 6 ------
 elf/setup-vdso.h | 4 ----
 include/link.h   | 2 --
 5 files changed, 3 insertions(+), 18 deletions(-)


base-commit: 6985865bc3ad5b23147ee73466583dd7fdf65892

Comments

Carlos O'Donell Sept. 8, 2023, 4:26 p.m. UTC | #1
On 9/8/23 07:20, Florian Weimer wrote:
> It is a left-over from commit 52a01100ad011293197637e42b5be1a479a2
> ("elf: Remove ad-hoc restrictions on dlopen callers [BZ #22787]").
> 
> When backporting commmit 6985865bc3ad5b23147ee73466583dd7fdf65892
> ("elf: Always call destructors in reverse constructor order
> (bug 30785)"), we can move the l_init_called_next field to this
> place, so that the internal GLIBC_PRIVATE ABI does not change.

LGTM.

No regression on x86_64.

On a whim I went to binutils/gdb to make sure they don't poke into the private link_map
and look at l_text_end for any reason and they don't, so we're clean there.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>

ABI impact is on _rtld_global due to embedded _dl_rtld_map.

No patches applied:
     8: 0000000000033000  4336 OBJECT  GLOBAL DEFAULT   17 _rtld_global@@GLIBC_PRIVATE
        - Original private ABI size.

 <2><64a>: Abbrev Number: 4 (DW_TAG_member)
    <64b>   DW_AT_name        : (indirect string, offset: 0x6d2): l_text_end
    <64f>   DW_AT_decl_file   : 9
    <64f>   DW_AT_decl_line   : 257
    <651>   DW_AT_decl_column : 16
    <652>   DW_AT_type        : <0x14a>
    <656>   DW_AT_data_member_location: 896
 <2><658>: Abbrev Number: 4 (DW_TAG_member)
    <659>   DW_AT_name        : (indirect string, offset: 0x778): l_scope_mem
    <65d>   DW_AT_decl_file   : 9
    <65d>   DW_AT_decl_line   : 260
    <65f>   DW_AT_decl_column : 26
    <660>   DW_AT_type        : <0xe5a>
    <664>   DW_AT_data_member_location: 904

Patch applied with entry removed:
     8: 0000000000033000  4328 OBJECT  GLOBAL DEFAULT   17 _rtld_global@@GLIBC_PRIVATE
        - Size is 8 bytes smaller.
	- Backport hazard: In place upgrades impacted.

Simulated backport with l_init_called_next pointer:
     8: 0000000000033000  4336 OBJECT  GLOBAL DEFAULT   17 _rtld_global@@GLIBC_PRIVATE
        - Size is the same as with no patch applied.

 <2><64a>: Abbrev Number: 4 (DW_TAG_member)
    <64b>   DW_AT_name        : (indirect string, offset: 0x40c): l_init_called_next2
    <64f>   DW_AT_decl_file   : 9
    <64f>   DW_AT_decl_line   : 256
    <651>   DW_AT_decl_column : 22
    <652>   DW_AT_type        : <0x7e1>
    <656>   DW_AT_data_member_location: 896
 <2><658>: Abbrev Number: 4 (DW_TAG_member)
    <659>   DW_AT_name        : (indirect string, offset: 0x781): l_scope_mem
    <65d>   DW_AT_decl_file   : 9
    <65d>   DW_AT_decl_line   : 259
    <65f>   DW_AT_decl_column : 26
    <660>   DW_AT_type        : <0xe5a>
    <664>   DW_AT_data_member_location: 904


> ---
>  elf/dl-load.c    | 2 +-
>  elf/dl-load.h    | 7 ++-----
>  elf/rtld.c       | 6 ------
>  elf/setup-vdso.h | 4 ----
>  include/link.h   | 2 --
>  5 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 9a87fda9c9..2923b1141d 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -1253,7 +1253,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>  
>      /* Now process the load commands and map segments into memory.
>         This is responsible for filling in:
> -       l_map_start, l_map_end, l_addr, l_contiguous, l_text_end, l_phdr
> +       l_map_start, l_map_end, l_addr, l_contiguous, l_phdr

OK.

>       */
>      errstring = _dl_map_segments (l, fd, header, type, loadcmds, nloadcmds,
>  				  maplength, has_holes, loader);
> diff --git a/elf/dl-load.h b/elf/dl-load.h
> index ecf6910c68..1d5207694b 100644
> --- a/elf/dl-load.h
> +++ b/elf/dl-load.h
> @@ -83,14 +83,11 @@ struct loadcmd
>  
>  /* This is a subroutine of _dl_map_segments.  It should be called for each
>     load command, some time after L->l_addr has been set correctly.  It is
> -   responsible for setting up the l_text_end and l_phdr fields.  */
> +   responsible for setting the l_phdr fields  */

OK.

>  static __always_inline void
>  _dl_postprocess_loadcmd (struct link_map *l, const ElfW(Ehdr) *header,
>                           const struct loadcmd *c)
>  {
> -  if (c->prot & PROT_EXEC)
> -    l->l_text_end = l->l_addr + c->mapend;
> -

OK.

>    if (l->l_phdr == 0
>        && c->mapoff <= header->e_phoff
>        && ((size_t) (c->mapend - c->mapstart + c->mapoff)
> @@ -103,7 +100,7 @@ _dl_postprocess_loadcmd (struct link_map *l, const ElfW(Ehdr) *header,
>  
>  /* This is a subroutine of _dl_map_object_from_fd.  It is responsible
>     for filling in several fields in *L: l_map_start, l_map_end, l_addr,
> -   l_contiguous, l_text_end, l_phdr.  On successful return, all the
> +   l_contiguous, l_phdr.  On successful return, all the

OK.

>     segments are mapped (or copied, or whatever) from the file into their
>     final places in the address space, with the correct page permissions,
>     and any bss-like regions already zeroed.  It returns a null pointer
> diff --git a/elf/rtld.c b/elf/rtld.c
> index a91e2a4471..5107d16fe3 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -477,7 +477,6 @@ _dl_start_final (void *arg, struct dl_start_final_info *info)
>    GL(dl_rtld_map).l_real = &GL(dl_rtld_map);
>    GL(dl_rtld_map).l_map_start = (ElfW(Addr)) &__ehdr_start;
>    GL(dl_rtld_map).l_map_end = (ElfW(Addr)) _end;
> -  GL(dl_rtld_map).l_text_end = (ElfW(Addr)) _etext;

OK.

>    /* Copy the TLS related data if necessary.  */
>  #ifndef DONT_USE_BOOTSTRAP_MAP
>  # if NO_TLS_OFFSET != 0
> @@ -1119,7 +1118,6 @@ rtld_setup_main_map (struct link_map *main_map)
>    bool has_interp = false;
>  
>    main_map->l_map_end = 0;
> -  main_map->l_text_end = 0;

OK.

>    /* Perhaps the executable has no PT_LOAD header entries at all.  */
>    main_map->l_map_start = ~0;
>    /* And it was opened directly.  */
> @@ -1211,8 +1209,6 @@ rtld_setup_main_map (struct link_map *main_map)
>  	  allocend = main_map->l_addr + ph->p_vaddr + ph->p_memsz;
>  	  if (main_map->l_map_end < allocend)
>  	    main_map->l_map_end = allocend;
> -	  if ((ph->p_flags & PF_X) && allocend > main_map->l_text_end)
> -	    main_map->l_text_end = allocend;

OK.

>  
>  	  /* The next expected address is the page following this load
>  	     segment.  */
> @@ -1272,8 +1268,6 @@ rtld_setup_main_map (struct link_map *main_map)
>        = (char *) main_map->l_tls_initimage + main_map->l_addr;
>    if (! main_map->l_map_end)
>      main_map->l_map_end = ~0;
> -  if (! main_map->l_text_end)
> -    main_map->l_text_end = ~0;

OK.

>    if (! GL(dl_rtld_map).l_libname && GL(dl_rtld_map).l_name)
>      {
>        /* We were invoked directly, so the program might not have a
> diff --git a/elf/setup-vdso.h b/elf/setup-vdso.h
> index 0079842d1f..d92b12a7aa 100644
> --- a/elf/setup-vdso.h
> +++ b/elf/setup-vdso.h
> @@ -51,9 +51,6 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
>  		l->l_addr = ph->p_vaddr;
>  	      if (ph->p_vaddr + ph->p_memsz >= l->l_map_end)
>  		l->l_map_end = ph->p_vaddr + ph->p_memsz;
> -	      if ((ph->p_flags & PF_X)
> -		  && ph->p_vaddr + ph->p_memsz >= l->l_text_end)
> -		l->l_text_end = ph->p_vaddr + ph->p_memsz;

OK.

>  	    }
>  	  else
>  	    /* There must be no TLS segment.  */
> @@ -62,7 +59,6 @@ setup_vdso (struct link_map *main_map __attribute__ ((unused)),
>        l->l_map_start = (ElfW(Addr)) GLRO(dl_sysinfo_dso);
>        l->l_addr = l->l_map_start - l->l_addr;
>        l->l_map_end += l->l_addr;
> -      l->l_text_end += l->l_addr;

OK.

>        l->l_ld = (void *) ((ElfW(Addr)) l->l_ld + l->l_addr);
>        elf_get_dynamic_info (l, false, false);
>        _dl_setup_hash (l);
> diff --git a/include/link.h b/include/link.h
> index 69bda3ed17..c6af095d87 100644
> --- a/include/link.h
> +++ b/include/link.h
> @@ -253,8 +253,6 @@ struct link_map
>      /* Start and finish of memory map for this object.  l_map_start
>         need not be the same as l_addr.  */
>      ElfW(Addr) l_map_start, l_map_end;
> -    /* End of the executable part of the mapping.  */
> -    ElfW(Addr) l_text_end;

OK. Backport hazard: internal ABI change to link_map impacting _rtld_global and in-place upgrades.

>  
>      /* Default array for 'l_scope'.  */
>      struct r_scope_elem *l_scope_mem[4];
> 
> base-commit: 6985865bc3ad5b23147ee73466583dd7fdf65892
>
diff mbox series

Patch

diff --git a/elf/dl-load.c b/elf/dl-load.c
index 9a87fda9c9..2923b1141d 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -1253,7 +1253,7 @@  _dl_map_object_from_fd (const char *name, const char *origname, int fd,
 
     /* Now process the load commands and map segments into memory.
        This is responsible for filling in:
-       l_map_start, l_map_end, l_addr, l_contiguous, l_text_end, l_phdr
+       l_map_start, l_map_end, l_addr, l_contiguous, l_phdr
      */
     errstring = _dl_map_segments (l, fd, header, type, loadcmds, nloadcmds,
 				  maplength, has_holes, loader);
diff --git a/elf/dl-load.h b/elf/dl-load.h
index ecf6910c68..1d5207694b 100644
--- a/elf/dl-load.h
+++ b/elf/dl-load.h
@@ -83,14 +83,11 @@  struct loadcmd
 
 /* This is a subroutine of _dl_map_segments.  It should be called for each
    load command, some time after L->l_addr has been set correctly.  It is
-   responsible for setting up the l_text_end and l_phdr fields.  */
+   responsible for setting the l_phdr fields  */
 static __always_inline void
 _dl_postprocess_loadcmd (struct link_map *l, const ElfW(Ehdr) *header,
                          const struct loadcmd *c)
 {
-  if (c->prot & PROT_EXEC)
-    l->l_text_end = l->l_addr + c->mapend;
-
   if (l->l_phdr == 0
       && c->mapoff <= header->e_phoff
       && ((size_t) (c->mapend - c->mapstart + c->mapoff)
@@ -103,7 +100,7 @@  _dl_postprocess_loadcmd (struct link_map *l, const ElfW(Ehdr) *header,
 
 /* This is a subroutine of _dl_map_object_from_fd.  It is responsible
    for filling in several fields in *L: l_map_start, l_map_end, l_addr,
-   l_contiguous, l_text_end, l_phdr.  On successful return, all the
+   l_contiguous, l_phdr.  On successful return, all the
    segments are mapped (or copied, or whatever) from the file into their
    final places in the address space, with the correct page permissions,
    and any bss-like regions already zeroed.  It returns a null pointer
diff --git a/elf/rtld.c b/elf/rtld.c
index a91e2a4471..5107d16fe3 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -477,7 +477,6 @@  _dl_start_final (void *arg, struct dl_start_final_info *info)
   GL(dl_rtld_map).l_real = &GL(dl_rtld_map);
   GL(dl_rtld_map).l_map_start = (ElfW(Addr)) &__ehdr_start;
   GL(dl_rtld_map).l_map_end = (ElfW(Addr)) _end;
-  GL(dl_rtld_map).l_text_end = (ElfW(Addr)) _etext;
   /* Copy the TLS related data if necessary.  */
 #ifndef DONT_USE_BOOTSTRAP_MAP
 # if NO_TLS_OFFSET != 0
@@ -1119,7 +1118,6 @@  rtld_setup_main_map (struct link_map *main_map)
   bool has_interp = false;
 
   main_map->l_map_end = 0;
-  main_map->l_text_end = 0;
   /* Perhaps the executable has no PT_LOAD header entries at all.  */
   main_map->l_map_start = ~0;
   /* And it was opened directly.  */
@@ -1211,8 +1209,6 @@  rtld_setup_main_map (struct link_map *main_map)
 	  allocend = main_map->l_addr + ph->p_vaddr + ph->p_memsz;
 	  if (main_map->l_map_end < allocend)
 	    main_map->l_map_end = allocend;
-	  if ((ph->p_flags & PF_X) && allocend > main_map->l_text_end)
-	    main_map->l_text_end = allocend;
 
 	  /* The next expected address is the page following this load
 	     segment.  */
@@ -1272,8 +1268,6 @@  rtld_setup_main_map (struct link_map *main_map)
       = (char *) main_map->l_tls_initimage + main_map->l_addr;
   if (! main_map->l_map_end)
     main_map->l_map_end = ~0;
-  if (! main_map->l_text_end)
-    main_map->l_text_end = ~0;
   if (! GL(dl_rtld_map).l_libname && GL(dl_rtld_map).l_name)
     {
       /* We were invoked directly, so the program might not have a
diff --git a/elf/setup-vdso.h b/elf/setup-vdso.h
index 0079842d1f..d92b12a7aa 100644
--- a/elf/setup-vdso.h
+++ b/elf/setup-vdso.h
@@ -51,9 +51,6 @@  setup_vdso (struct link_map *main_map __attribute__ ((unused)),
 		l->l_addr = ph->p_vaddr;
 	      if (ph->p_vaddr + ph->p_memsz >= l->l_map_end)
 		l->l_map_end = ph->p_vaddr + ph->p_memsz;
-	      if ((ph->p_flags & PF_X)
-		  && ph->p_vaddr + ph->p_memsz >= l->l_text_end)
-		l->l_text_end = ph->p_vaddr + ph->p_memsz;
 	    }
 	  else
 	    /* There must be no TLS segment.  */
@@ -62,7 +59,6 @@  setup_vdso (struct link_map *main_map __attribute__ ((unused)),
       l->l_map_start = (ElfW(Addr)) GLRO(dl_sysinfo_dso);
       l->l_addr = l->l_map_start - l->l_addr;
       l->l_map_end += l->l_addr;
-      l->l_text_end += l->l_addr;
       l->l_ld = (void *) ((ElfW(Addr)) l->l_ld + l->l_addr);
       elf_get_dynamic_info (l, false, false);
       _dl_setup_hash (l);
diff --git a/include/link.h b/include/link.h
index 69bda3ed17..c6af095d87 100644
--- a/include/link.h
+++ b/include/link.h
@@ -253,8 +253,6 @@  struct link_map
     /* Start and finish of memory map for this object.  l_map_start
        need not be the same as l_addr.  */
     ElfW(Addr) l_map_start, l_map_end;
-    /* End of the executable part of the mapping.  */
-    ElfW(Addr) l_text_end;
 
     /* Default array for 'l_scope'.  */
     struct r_scope_elem *l_scope_mem[4];