* [PATCH] elf: Remove unused l_text_end field from struct link_map
@ 2023-09-08 11:20 Florian Weimer
2023-09-08 16:26 ` Carlos O'Donell
0 siblings, 1 reply; 2+ messages in thread
From: Florian Weimer @ 2023-09-08 11:20 UTC (permalink / raw)
To: libc-alpha
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(-)
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];
base-commit: 6985865bc3ad5b23147ee73466583dd7fdf65892
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] elf: Remove unused l_text_end field from struct link_map
2023-09-08 11:20 [PATCH] elf: Remove unused l_text_end field from struct link_map Florian Weimer
@ 2023-09-08 16:26 ` Carlos O'Donell
0 siblings, 0 replies; 2+ messages in thread
From: Carlos O'Donell @ 2023-09-08 16:26 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
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
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-09-08 16:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-08 11:20 [PATCH] elf: Remove unused l_text_end field from struct link_map Florian Weimer
2023-09-08 16:26 ` Carlos O'Donell
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).