* [PATCH 0/3] Fix elf/tst-dl_find_objects with --enable-hardcoded-path-in-tests
@ 2022-01-03 17:11 Florian Weimer
2022-01-03 17:11 ` [PATCH 1/3] elf: Introduce rtld_setup_main_map Florian Weimer
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Florian Weimer @ 2022-01-03 17:11 UTC (permalink / raw)
To: libc-alpha
This mini-series reworks the way we we set up l_contiguous for the main
mapping. This uncovered an oddity in some binaries, tracked as bug
28743. Due to the oddity, I do not want to assert that the main mapping
is contiguous (on most targets).
Thanks,
Florian
Florian Weimer (3):
elf: Introduce rtld_setup_main_map
elf: Set l_contiguous to 1 for the main map in more cases
elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug
28732)
elf/rtld.c | 328 ++++++++++++++++++++++-----------------
elf/tst-dl_find_object.c | 29 ++--
2 files changed, 201 insertions(+), 156 deletions(-)
base-commit: a51faeee6ae68da63e65eb0a1eb6c9ec2ce2148b
--
2.33.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/3] elf: Introduce rtld_setup_main_map
2022-01-03 17:11 [PATCH 0/3] Fix elf/tst-dl_find_objects with --enable-hardcoded-path-in-tests Florian Weimer
@ 2022-01-03 17:11 ` Florian Weimer
2022-01-16 0:11 ` H.J. Lu
2022-01-03 17:11 ` [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases Florian Weimer
2022-01-03 17:11 ` [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732) Florian Weimer
2 siblings, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-03 17:11 UTC (permalink / raw)
To: libc-alpha
This function collects most of the processing needed to initialize
the link map for the main executable.
---
elf/rtld.c | 303 ++++++++++++++++++++++++++++-------------------------
1 file changed, 159 insertions(+), 144 deletions(-)
diff --git a/elf/rtld.c b/elf/rtld.c
index 24e48bf3fa..ba6e31377d 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1126,17 +1126,172 @@ rtld_chain_load (struct link_map *main_map, char *argv0)
rtld_soname, pathname, errcode);
}
+/* Called to complete the initialization of the link map for the main
+ executable. Returns true if there is a PT_INTERP segment. */
+static bool
+rtld_setup_main_map (struct link_map *main_map)
+{
+ /* This have already been filled in right after _dl_new_object, or
+ as part of _dl_map_object. */
+ const ElfW(Phdr) *phdr = main_map->l_phdr;
+ ElfW(Word) phnum = main_map->l_phnum;
+
+ 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. */
+ ++main_map->l_direct_opencount;
+
+ /* Scan the program header table for the dynamic section. */
+ for (const ElfW(Phdr) *ph = phdr; ph < &phdr[phnum]; ++ph)
+ switch (ph->p_type)
+ {
+ case PT_PHDR:
+ /* Find out the load address. */
+ main_map->l_addr = (ElfW(Addr)) phdr - ph->p_vaddr;
+ break;
+ case PT_DYNAMIC:
+ /* This tells us where to find the dynamic section,
+ which tells us everything we need to do. */
+ main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
+ main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
+ break;
+ case PT_INTERP:
+ /* This "interpreter segment" was used by the program loader to
+ find the program interpreter, which is this program itself, the
+ dynamic linker. We note what name finds us, so that a future
+ dlopen call or DT_NEEDED entry, for something that wants to link
+ against the dynamic linker as a shared library, will know that
+ the shared object is already loaded. */
+ _dl_rtld_libname.name = ((const char *) main_map->l_addr
+ + ph->p_vaddr);
+ /* _dl_rtld_libname.next = NULL; Already zero. */
+ GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
+
+ /* Ordinarilly, we would get additional names for the loader from
+ our DT_SONAME. This can't happen if we were actually linked as
+ a static executable (detect this case when we have no DYNAMIC).
+ If so, assume the filename component of the interpreter path to
+ be our SONAME, and add it to our name list. */
+ if (GL(dl_rtld_map).l_ld == NULL)
+ {
+ const char *p = NULL;
+ const char *cp = _dl_rtld_libname.name;
+
+ /* Find the filename part of the path. */
+ while (*cp != '\0')
+ if (*cp++ == '/')
+ p = cp;
+
+ if (p != NULL)
+ {
+ _dl_rtld_libname2.name = p;
+ /* _dl_rtld_libname2.next = NULL; Already zero. */
+ _dl_rtld_libname.next = &_dl_rtld_libname2;
+ }
+ }
+
+ has_interp = true;
+ break;
+ case PT_LOAD:
+ {
+ ElfW(Addr) mapstart;
+ ElfW(Addr) allocend;
+
+ /* Remember where the main program starts in memory. */
+ mapstart = (main_map->l_addr
+ + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
+ if (main_map->l_map_start > mapstart)
+ main_map->l_map_start = mapstart;
+
+ /* Also where it ends. */
+ 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;
+ }
+ break;
+
+ case PT_TLS:
+ if (ph->p_memsz > 0)
+ {
+ /* Note that in the case the dynamic linker we duplicate work
+ here since we read the PT_TLS entry already in
+ _dl_start_final. But the result is repeatable so do not
+ check for this special but unimportant case. */
+ main_map->l_tls_blocksize = ph->p_memsz;
+ main_map->l_tls_align = ph->p_align;
+ if (ph->p_align == 0)
+ main_map->l_tls_firstbyte_offset = 0;
+ else
+ main_map->l_tls_firstbyte_offset = (ph->p_vaddr
+ & (ph->p_align - 1));
+ main_map->l_tls_initimage_size = ph->p_filesz;
+ main_map->l_tls_initimage = (void *) ph->p_vaddr;
+
+ /* This image gets the ID one. */
+ GL(dl_tls_max_dtv_idx) = main_map->l_tls_modid = 1;
+ }
+ break;
+
+ case PT_GNU_STACK:
+ GL(dl_stack_flags) = ph->p_flags;
+ break;
+
+ case PT_GNU_RELRO:
+ main_map->l_relro_addr = ph->p_vaddr;
+ main_map->l_relro_size = ph->p_memsz;
+ break;
+ }
+ /* Process program headers again, but scan them backwards so
+ that PT_NOTE can be skipped if PT_GNU_PROPERTY exits. */
+ for (const ElfW(Phdr) *ph = &phdr[phnum]; ph != phdr; --ph)
+ switch (ph[-1].p_type)
+ {
+ case PT_NOTE:
+ _dl_process_pt_note (main_map, -1, &ph[-1]);
+ break;
+ case PT_GNU_PROPERTY:
+ _dl_process_pt_gnu_property (main_map, -1, &ph[-1]);
+ break;
+ }
+
+ /* Adjust the address of the TLS initialization image in case
+ the executable is actually an ET_DYN object. */
+ if (main_map->l_tls_initimage != NULL)
+ main_map->l_tls_initimage
+ = (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
+ PT_INTERP. */
+ _dl_rtld_libname.name = GL(dl_rtld_map).l_name;
+ /* _dl_rtld_libname.next = NULL; Already zero. */
+ GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
+ }
+ else
+ assert (GL(dl_rtld_map).l_libname); /* How else did we get here? */
+
+ return has_interp;
+}
+
static void
dl_main (const ElfW(Phdr) *phdr,
ElfW(Word) phnum,
ElfW(Addr) *user_entry,
ElfW(auxv_t) *auxv)
{
- const ElfW(Phdr) *ph;
struct link_map *main_map;
size_t file_size;
char *file;
- bool has_interp = false;
unsigned int i;
bool prelinked = false;
bool rtld_is_main = false;
@@ -1350,7 +1505,7 @@ dl_main (const ElfW(Phdr) *phdr,
load the program below unless it has a PT_GNU_STACK indicating
nonexecutable stack is ok. */
- for (ph = phdr; ph < &phdr[phnum]; ++ph)
+ for (const ElfW(Phdr) *ph = phdr; ph < &phdr[phnum]; ++ph)
if (ph->p_type == PT_GNU_STACK)
{
GL(dl_stack_flags) = ph->p_flags;
@@ -1469,147 +1624,7 @@ dl_main (const ElfW(Phdr) *phdr,
information for the program. */
}
- 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. */
- ++main_map->l_direct_opencount;
-
- /* Scan the program header table for the dynamic section. */
- for (ph = phdr; ph < &phdr[phnum]; ++ph)
- switch (ph->p_type)
- {
- case PT_PHDR:
- /* Find out the load address. */
- main_map->l_addr = (ElfW(Addr)) phdr - ph->p_vaddr;
- break;
- case PT_DYNAMIC:
- /* This tells us where to find the dynamic section,
- which tells us everything we need to do. */
- main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
- main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
- break;
- case PT_INTERP:
- /* This "interpreter segment" was used by the program loader to
- find the program interpreter, which is this program itself, the
- dynamic linker. We note what name finds us, so that a future
- dlopen call or DT_NEEDED entry, for something that wants to link
- against the dynamic linker as a shared library, will know that
- the shared object is already loaded. */
- _dl_rtld_libname.name = ((const char *) main_map->l_addr
- + ph->p_vaddr);
- /* _dl_rtld_libname.next = NULL; Already zero. */
- GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
-
- /* Ordinarilly, we would get additional names for the loader from
- our DT_SONAME. This can't happen if we were actually linked as
- a static executable (detect this case when we have no DYNAMIC).
- If so, assume the filename component of the interpreter path to
- be our SONAME, and add it to our name list. */
- if (GL(dl_rtld_map).l_ld == NULL)
- {
- const char *p = NULL;
- const char *cp = _dl_rtld_libname.name;
-
- /* Find the filename part of the path. */
- while (*cp != '\0')
- if (*cp++ == '/')
- p = cp;
-
- if (p != NULL)
- {
- _dl_rtld_libname2.name = p;
- /* _dl_rtld_libname2.next = NULL; Already zero. */
- _dl_rtld_libname.next = &_dl_rtld_libname2;
- }
- }
-
- has_interp = true;
- break;
- case PT_LOAD:
- {
- ElfW(Addr) mapstart;
- ElfW(Addr) allocend;
-
- /* Remember where the main program starts in memory. */
- mapstart = (main_map->l_addr
- + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
- if (main_map->l_map_start > mapstart)
- main_map->l_map_start = mapstart;
-
- /* Also where it ends. */
- 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;
- }
- break;
-
- case PT_TLS:
- if (ph->p_memsz > 0)
- {
- /* Note that in the case the dynamic linker we duplicate work
- here since we read the PT_TLS entry already in
- _dl_start_final. But the result is repeatable so do not
- check for this special but unimportant case. */
- main_map->l_tls_blocksize = ph->p_memsz;
- main_map->l_tls_align = ph->p_align;
- if (ph->p_align == 0)
- main_map->l_tls_firstbyte_offset = 0;
- else
- main_map->l_tls_firstbyte_offset = (ph->p_vaddr
- & (ph->p_align - 1));
- main_map->l_tls_initimage_size = ph->p_filesz;
- main_map->l_tls_initimage = (void *) ph->p_vaddr;
-
- /* This image gets the ID one. */
- GL(dl_tls_max_dtv_idx) = main_map->l_tls_modid = 1;
- }
- break;
-
- case PT_GNU_STACK:
- GL(dl_stack_flags) = ph->p_flags;
- break;
-
- case PT_GNU_RELRO:
- main_map->l_relro_addr = ph->p_vaddr;
- main_map->l_relro_size = ph->p_memsz;
- break;
- }
- /* Process program headers again, but scan them backwards so
- that PT_NOTE can be skipped if PT_GNU_PROPERTY exits. */
- for (ph = &phdr[phnum]; ph != phdr; --ph)
- switch (ph[-1].p_type)
- {
- case PT_NOTE:
- _dl_process_pt_note (main_map, -1, &ph[-1]);
- break;
- case PT_GNU_PROPERTY:
- _dl_process_pt_gnu_property (main_map, -1, &ph[-1]);
- break;
- }
-
- /* Adjust the address of the TLS initialization image in case
- the executable is actually an ET_DYN object. */
- if (main_map->l_tls_initimage != NULL)
- main_map->l_tls_initimage
- = (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
- PT_INTERP. */
- _dl_rtld_libname.name = GL(dl_rtld_map).l_name;
- /* _dl_rtld_libname.next = NULL; Already zero. */
- GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
- }
- else
- assert (GL(dl_rtld_map).l_libname); /* How else did we get here? */
+ bool has_interp = rtld_setup_main_map (main_map);
/* If the current libname is different from the SONAME, add the
latter as well. */
--
2.33.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
2022-01-03 17:11 [PATCH 0/3] Fix elf/tst-dl_find_objects with --enable-hardcoded-path-in-tests Florian Weimer
2022-01-03 17:11 ` [PATCH 1/3] elf: Introduce rtld_setup_main_map Florian Weimer
@ 2022-01-03 17:11 ` Florian Weimer
2022-01-16 0:10 ` H.J. Lu
2022-01-03 17:11 ` [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732) Florian Weimer
2 siblings, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-03 17:11 UTC (permalink / raw)
To: libc-alpha
l_contiguous was not initialized at all for the main map and
always 0. This commit adds code to check if the LOAD segments
are adjacent to each other, and sets l_contiguous accordingly.
This helps _dl_find_object because it is more efficient if the
main mapping is contiguous.
Note that not all (PIE or non-PIE) binaries are contiguous in this
way because BFD ld creates executables with LOAD holes:
ELF LOAD segments creating holes in the process image on GNU/Linux
https://sourceware.org/pipermail/binutils/2022-January/119082.html
https://sourceware.org/bugzilla/show_bug.cgi?id=28743
---
elf/rtld.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/elf/rtld.c b/elf/rtld.c
index ba6e31377d..53293f5b13 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1144,6 +1144,22 @@ rtld_setup_main_map (struct link_map *main_map)
main_map->l_map_start = ~0;
/* And it was opened directly. */
++main_map->l_direct_opencount;
+ main_map->l_contiguous = 1;
+
+ /* A PT_LOAD segment at an unexpected address will clear the
+ l_contiguous flag. The ELF specification says that PT_LOAD
+ segments need to be sorted in in increasing order, but perhaps
+ not all executables follow this requirement. Having l_contiguous
+ equal to 1 is just an optimization, so the code below does not
+ try to sort the segments in case they are unordered.
+
+ There is one corner case in which l_contiguous is not set to 1,
+ but where it could be set: If a PIE (ET_DYN) binary is loaded by
+ glibc itself (not the kernel), it is always contiguous due to the
+ way the glibc loader works. However, the kernel loader may still
+ create holes in this case, and the code here still uses 0
+ conservatively for the glibc-loaded case, too. */
+ ElfW(Addr) expected_load_address = 0;
/* Scan the program header table for the dynamic section. */
for (const ElfW(Phdr) *ph = phdr; ph < &phdr[phnum]; ++ph)
@@ -1207,12 +1223,21 @@ rtld_setup_main_map (struct link_map *main_map)
if (main_map->l_map_start > mapstart)
main_map->l_map_start = mapstart;
+ if (main_map->l_contiguous && expected_load_address != 0
+ && expected_load_address != mapstart)
+ main_map->l_contiguous = 0;
+
/* Also where it ends. */
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. */
+ expected_load_address = ((allocend + GLRO(dl_pagesize) - 1)
+ & ~(GLRO(dl_pagesize) - 1));
}
break;
--
2.33.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-03 17:11 [PATCH 0/3] Fix elf/tst-dl_find_objects with --enable-hardcoded-path-in-tests Florian Weimer
2022-01-03 17:11 ` [PATCH 1/3] elf: Introduce rtld_setup_main_map Florian Weimer
2022-01-03 17:11 ` [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases Florian Weimer
@ 2022-01-03 17:11 ` Florian Weimer
2022-01-14 15:06 ` H.J. Lu
2022-01-16 14:05 ` H.J. Lu
2 siblings, 2 replies; 18+ messages in thread
From: Florian Weimer @ 2022-01-03 17:11 UTC (permalink / raw)
To: libc-alpha
---
elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
index 21cdc0f848..2ad1924088 100644
--- a/elf/tst-dl_find_object.c
+++ b/elf/tst-dl_find_object.c
@@ -71,19 +71,24 @@ check (void *address,
__FILE__, line, address,
actual.dlfo_flags, expected->dlfo_flags);
}
- if (actual.dlfo_flags != expected->dlfo_flags)
+ if (expected->dlfo_link_map->l_contiguous)
{
- support_record_failure ();
- printf ("%s:%d: error: %p: map start is %p, expected %p\n",
- __FILE__, line,
- address, actual.dlfo_map_start, expected->dlfo_map_start);
- }
- if (actual.dlfo_map_end != expected->dlfo_map_end)
- {
- support_record_failure ();
- printf ("%s:%d: error: %p: map end is %p, expected %p\n",
- __FILE__, line,
- address, actual.dlfo_map_end, expected->dlfo_map_end);
+ /* If the mappings are not contiguous, the actual and execpted
+ mappings may differ, so this subtest will not work. */
+ if (actual.dlfo_flags != expected->dlfo_flags)
+ {
+ support_record_failure ();
+ printf ("%s:%d: error: %p: map start is %p, expected %p\n",
+ __FILE__, line,
+ address, actual.dlfo_map_start, expected->dlfo_map_start);
+ }
+ if (actual.dlfo_map_end != expected->dlfo_map_end)
+ {
+ support_record_failure ();
+ printf ("%s:%d: error: %p: map end is %p, expected %p\n",
+ __FILE__, line,
+ address, actual.dlfo_map_end, expected->dlfo_map_end);
+ }
}
if (actual.dlfo_link_map != expected->dlfo_link_map)
{
--
2.33.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-03 17:11 ` [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732) Florian Weimer
@ 2022-01-14 15:06 ` H.J. Lu
2022-01-14 15:09 ` H.J. Lu
2022-01-14 15:10 ` Florian Weimer
2022-01-16 14:05 ` H.J. Lu
1 sibling, 2 replies; 18+ messages in thread
From: H.J. Lu @ 2022-01-14 15:06 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> ---
> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> index 21cdc0f848..2ad1924088 100644
> --- a/elf/tst-dl_find_object.c
> +++ b/elf/tst-dl_find_object.c
> @@ -71,19 +71,24 @@ check (void *address,
> __FILE__, line, address,
> actual.dlfo_flags, expected->dlfo_flags);
> }
> - if (actual.dlfo_flags != expected->dlfo_flags)
> + if (expected->dlfo_link_map->l_contiguous)
> {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_start, expected->dlfo_map_start);
> - }
> - if (actual.dlfo_map_end != expected->dlfo_map_end)
> - {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_end, expected->dlfo_map_end);
> + /* If the mappings are not contiguous, the actual and execpted
> + mappings may differ, so this subtest will not work. */
> + if (actual.dlfo_flags != expected->dlfo_flags)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_start, expected->dlfo_map_start);
> + }
> + if (actual.dlfo_map_end != expected->dlfo_map_end)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_end, expected->dlfo_map_end);
> + }
> }
> if (actual.dlfo_link_map != expected->dlfo_link_map)
> {
> --
> 2.33.1
>
I still see
FAIL: elf/tst-dl_find_object
even when using the new linker with the fix for
https://sourceware.org/bugzilla/show_bug.cgi?id=28743
to remove the 1-page gap. Which file doesn't have
non-contiguous mapping?
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:06 ` H.J. Lu
@ 2022-01-14 15:09 ` H.J. Lu
2022-01-14 15:10 ` Florian Weimer
1 sibling, 0 replies; 18+ messages in thread
From: H.J. Lu @ 2022-01-14 15:09 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Fri, Jan 14, 2022 at 7:06 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > ---
> > elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> > 1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> > index 21cdc0f848..2ad1924088 100644
> > --- a/elf/tst-dl_find_object.c
> > +++ b/elf/tst-dl_find_object.c
> > @@ -71,19 +71,24 @@ check (void *address,
> > __FILE__, line, address,
> > actual.dlfo_flags, expected->dlfo_flags);
> > }
> > - if (actual.dlfo_flags != expected->dlfo_flags)
> > + if (expected->dlfo_link_map->l_contiguous)
> > {
> > - support_record_failure ();
> > - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> > - __FILE__, line,
> > - address, actual.dlfo_map_start, expected->dlfo_map_start);
> > - }
> > - if (actual.dlfo_map_end != expected->dlfo_map_end)
> > - {
> > - support_record_failure ();
> > - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> > - __FILE__, line,
> > - address, actual.dlfo_map_end, expected->dlfo_map_end);
> > + /* If the mappings are not contiguous, the actual and execpted
> > + mappings may differ, so this subtest will not work. */
> > + if (actual.dlfo_flags != expected->dlfo_flags)
> > + {
> > + support_record_failure ();
> > + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> > + __FILE__, line,
> > + address, actual.dlfo_map_start, expected->dlfo_map_start);
> > + }
> > + if (actual.dlfo_map_end != expected->dlfo_map_end)
> > + {
> > + support_record_failure ();
> > + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> > + __FILE__, line,
> > + address, actual.dlfo_map_end, expected->dlfo_map_end);
> > + }
> > }
> > if (actual.dlfo_link_map != expected->dlfo_link_map)
> > {
> > --
> > 2.33.1
> >
>
> I still see
>
> FAIL: elf/tst-dl_find_object
>
> even when using the new linker with the fix for
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28743
>
> to remove the 1-page gap. Which file doesn't have
> non-contiguous mapping?
>
The linker bug isn't really fixed:
[21] .eh_frame PROGBITS 0000000000005ef0 005ef0
000758 00 A 0 0 8
[22] .init_array INIT_ARRAY 0000000000007000 007000
000010 08 WA 0 0 8
[23] .fini_array FINI_ARRAY 0000000000007010 007010
000008 08 WA 0 0 8
[24] .data.rel.ro PROGBITS 0000000000007020 007020
0000a0 00 WA 0 0 32
[25] .dynamic DYNAMIC 00000000000070c0 0070c0
000200 10 WA 8 0 8
[26] .got PROGBITS 00000000000072c0 0072c0
000078 08 WA 0 0 8
[27] .got.plt PROGBITS 0000000000008000 008000
0001d0 08 WA 0 0 8
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:06 ` H.J. Lu
2022-01-14 15:09 ` H.J. Lu
@ 2022-01-14 15:10 ` Florian Weimer
2022-01-14 15:19 ` H.J. Lu
1 sibling, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-14 15:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
* H. J. Lu:
> On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> ---
>> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
>> 1 file changed, 17 insertions(+), 12 deletions(-)
>>
>> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
>> index 21cdc0f848..2ad1924088 100644
>> --- a/elf/tst-dl_find_object.c
>> +++ b/elf/tst-dl_find_object.c
>> @@ -71,19 +71,24 @@ check (void *address,
>> __FILE__, line, address,
>> actual.dlfo_flags, expected->dlfo_flags);
>> }
>> - if (actual.dlfo_flags != expected->dlfo_flags)
>> + if (expected->dlfo_link_map->l_contiguous)
>> {
>> - support_record_failure ();
>> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
>> - __FILE__, line,
>> - address, actual.dlfo_map_start, expected->dlfo_map_start);
>> - }
>> - if (actual.dlfo_map_end != expected->dlfo_map_end)
>> - {
>> - support_record_failure ();
>> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
>> - __FILE__, line,
>> - address, actual.dlfo_map_end, expected->dlfo_map_end);
>> + /* If the mappings are not contiguous, the actual and execpted
>> + mappings may differ, so this subtest will not work. */
>> + if (actual.dlfo_flags != expected->dlfo_flags)
>> + {
>> + support_record_failure ();
>> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
>> + __FILE__, line,
>> + address, actual.dlfo_map_start, expected->dlfo_map_start);
>> + }
>> + if (actual.dlfo_map_end != expected->dlfo_map_end)
>> + {
>> + support_record_failure ();
>> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
>> + __FILE__, line,
>> + address, actual.dlfo_map_end, expected->dlfo_map_end);
>> + }
>> }
>> if (actual.dlfo_link_map != expected->dlfo_link_map)
>> {
>> --
>> 2.33.1
>>
>
> I still see
>
> FAIL: elf/tst-dl_find_object
>
> even when using the new linker with the fix for
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28743
>
> to remove the 1-page gap. Which file doesn't have
> non-contiguous mapping?
We never set l_contiguous for the main executable, so it doesn't matter
what the link editor does. And none of the glibc fixes went in so far.
Thanks,
Florian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:10 ` Florian Weimer
@ 2022-01-14 15:19 ` H.J. Lu
2022-01-14 15:39 ` Florian Weimer
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2022-01-14 15:19 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Fri, Jan 14, 2022 at 7:10 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> ---
> >> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> >> 1 file changed, 17 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> >> index 21cdc0f848..2ad1924088 100644
> >> --- a/elf/tst-dl_find_object.c
> >> +++ b/elf/tst-dl_find_object.c
> >> @@ -71,19 +71,24 @@ check (void *address,
> >> __FILE__, line, address,
> >> actual.dlfo_flags, expected->dlfo_flags);
> >> }
> >> - if (actual.dlfo_flags != expected->dlfo_flags)
> >> + if (expected->dlfo_link_map->l_contiguous)
> >> {
> >> - support_record_failure ();
> >> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> >> - __FILE__, line,
> >> - address, actual.dlfo_map_start, expected->dlfo_map_start);
> >> - }
> >> - if (actual.dlfo_map_end != expected->dlfo_map_end)
> >> - {
> >> - support_record_failure ();
> >> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> >> - __FILE__, line,
> >> - address, actual.dlfo_map_end, expected->dlfo_map_end);
> >> + /* If the mappings are not contiguous, the actual and execpted
> >> + mappings may differ, so this subtest will not work. */
> >> + if (actual.dlfo_flags != expected->dlfo_flags)
> >> + {
> >> + support_record_failure ();
> >> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> >> + __FILE__, line,
> >> + address, actual.dlfo_map_start, expected->dlfo_map_start);
> >> + }
> >> + if (actual.dlfo_map_end != expected->dlfo_map_end)
> >> + {
> >> + support_record_failure ();
> >> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> >> + __FILE__, line,
> >> + address, actual.dlfo_map_end, expected->dlfo_map_end);
> >> + }
> >> }
> >> if (actual.dlfo_link_map != expected->dlfo_link_map)
> >> {
> >> --
> >> 2.33.1
> >>
> >
> > I still see
> >
> > FAIL: elf/tst-dl_find_object
> >
> > even when using the new linker with the fix for
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=28743
> >
> > to remove the 1-page gap. Which file doesn't have
> > non-contiguous mapping?
>
> We never set l_contiguous for the main executable, so it doesn't matter
> what the link editor does. And none of the glibc fixes went in so far.
>
> Thanks,
> Florian
>
Should l_contiguous be set on the main executable? If not, why?
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:19 ` H.J. Lu
@ 2022-01-14 15:39 ` Florian Weimer
2022-01-14 15:47 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-14 15:39 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
* H. J. Lu:
>> We never set l_contiguous for the main executable, so it doesn't matter
>> what the link editor does. And none of the glibc fixes went in so far.
>>
>> Thanks,
>> Florian
>>
>
> Should l_contiguous be set on the main executable? If not, why?
I think it should be set even if the kernel loads the main executable.
See patch 2:
[PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
<https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
Thanks,
Florian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:39 ` Florian Weimer
@ 2022-01-14 15:47 ` H.J. Lu
2022-01-14 15:51 ` Florian Weimer
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2022-01-14 15:47 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> >> We never set l_contiguous for the main executable, so it doesn't matter
> >> what the link editor does. And none of the glibc fixes went in so far.
> >>
> >> Thanks,
> >> Florian
> >>
> >
> > Should l_contiguous be set on the main executable? If not, why?
>
> I think it should be set even if the kernel loads the main executable.
> See patch 2:
>
> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
If we have the second patch, do we still need the 3rd one?
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:47 ` H.J. Lu
@ 2022-01-14 15:51 ` Florian Weimer
2022-01-14 15:54 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-14 15:51 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
* H. J. Lu:
> On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> >> We never set l_contiguous for the main executable, so it doesn't matter
>> >> what the link editor does. And none of the glibc fixes went in so far.
>> >>
>> >> Thanks,
>> >> Florian
>> >>
>> >
>> > Should l_contiguous be set on the main executable? If not, why?
>>
>> I think it should be set even if the kernel loads the main executable.
>> See patch 2:
>>
>> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
>> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
>
> If we have the second patch, do we still need the 3rd one?
I think gaps are expected on some architectures for main executable
(ia64?).
Thanks,
Florian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:51 ` Florian Weimer
@ 2022-01-14 15:54 ` H.J. Lu
2022-01-14 15:56 ` Florian Weimer
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2022-01-14 15:54 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Fri, Jan 14, 2022 at 7:51 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> >> We never set l_contiguous for the main executable, so it doesn't matter
> >> >> what the link editor does. And none of the glibc fixes went in so far.
> >> >>
> >> >> Thanks,
> >> >> Florian
> >> >>
> >> >
> >> > Should l_contiguous be set on the main executable? If not, why?
> >>
> >> I think it should be set even if the kernel loads the main executable.
> >> See patch 2:
> >>
> >> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
> >> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
> >
> > If we have the second patch, do we still need the 3rd one?
>
> I think gaps are expected on some architectures for main executable
> (ia64?).
>
Please add a header to indicate that the gap is expected only on some
architectures.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:54 ` H.J. Lu
@ 2022-01-14 15:56 ` Florian Weimer
2022-01-14 16:06 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-14 15:56 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
* H. J. Lu:
> On Fri, Jan 14, 2022 at 7:51 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>> > On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
>> >>
>> >> * H. J. Lu:
>> >>
>> >> >> We never set l_contiguous for the main executable, so it doesn't matter
>> >> >> what the link editor does. And none of the glibc fixes went in so far.
>> >> >>
>> >> >> Thanks,
>> >> >> Florian
>> >> >>
>> >> >
>> >> > Should l_contiguous be set on the main executable? If not, why?
>> >>
>> >> I think it should be set even if the kernel loads the main executable.
>> >> See patch 2:
>> >>
>> >> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
>> >> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
>> >
>> > If we have the second patch, do we still need the 3rd one?
>>
>> I think gaps are expected on some architectures for main executable
>> (ia64?).
>>
>
> Please add a header to indicate that the gap is expected only on some
> architectures.
Sorry, do you mean there should be a .h file to indicate whether the
architecture expects a gap?
How do I tell whether the gap is expected or the result of binutils
bugs?
Thanks,
Florian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 15:56 ` Florian Weimer
@ 2022-01-14 16:06 ` H.J. Lu
2022-01-14 16:12 ` Florian Weimer
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2022-01-14 16:06 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Fri, Jan 14, 2022 at 7:56 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > On Fri, Jan 14, 2022 at 7:51 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > On Fri, Jan 14, 2022 at 7:39 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> >>
> >> >> * H. J. Lu:
> >> >>
> >> >> >> We never set l_contiguous for the main executable, so it doesn't matter
> >> >> >> what the link editor does. And none of the glibc fixes went in so far.
> >> >> >>
> >> >> >> Thanks,
> >> >> >> Florian
> >> >> >>
> >> >> >
> >> >> > Should l_contiguous be set on the main executable? If not, why?
> >> >>
> >> >> I think it should be set even if the kernel loads the main executable.
> >> >> See patch 2:
> >> >>
> >> >> [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
> >> >> <https://sourceware.org/pipermail/libc-alpha/2022-January/134894.html>
> >> >
> >> > If we have the second patch, do we still need the 3rd one?
> >>
> >> I think gaps are expected on some architectures for main executable
> >> (ia64?).
> >>
> >
> > Please add a header to indicate that the gap is expected only on some
> > architectures.
>
> Sorry, do you mean there should be a .h file to indicate whether the
> architecture expects a gap?
>
> How do I tell whether the gap is expected or the result of binutils
> bugs?
>
You can add a linker test to check for the linker bug.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-14 16:06 ` H.J. Lu
@ 2022-01-14 16:12 ` Florian Weimer
0 siblings, 0 replies; 18+ messages in thread
From: Florian Weimer @ 2022-01-14 16:12 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
* H. J. Lu:
> You can add a linker test to check for the linker bug.
The linker bug could appear randomly, depending on the input files and
their contents.
Thanks,
Florian
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases
2022-01-03 17:11 ` [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases Florian Weimer
@ 2022-01-16 0:10 ` H.J. Lu
0 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 2022-01-16 0:10 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Mon, Jan 3, 2022 at 9:12 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> l_contiguous was not initialized at all for the main map and
> always 0. This commit adds code to check if the LOAD segments
> are adjacent to each other, and sets l_contiguous accordingly.
> This helps _dl_find_object because it is more efficient if the
> main mapping is contiguous.
>
> Note that not all (PIE or non-PIE) binaries are contiguous in this
> way because BFD ld creates executables with LOAD holes:
>
> ELF LOAD segments creating holes in the process image on GNU/Linux
> https://sourceware.org/pipermail/binutils/2022-January/119082.html
> https://sourceware.org/bugzilla/show_bug.cgi?id=28743
> ---
> elf/rtld.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/elf/rtld.c b/elf/rtld.c
> index ba6e31377d..53293f5b13 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1144,6 +1144,22 @@ rtld_setup_main_map (struct link_map *main_map)
> main_map->l_map_start = ~0;
> /* And it was opened directly. */
> ++main_map->l_direct_opencount;
> + main_map->l_contiguous = 1;
> +
> + /* A PT_LOAD segment at an unexpected address will clear the
> + l_contiguous flag. The ELF specification says that PT_LOAD
> + segments need to be sorted in in increasing order, but perhaps
> + not all executables follow this requirement. Having l_contiguous
> + equal to 1 is just an optimization, so the code below does not
> + try to sort the segments in case they are unordered.
> +
> + There is one corner case in which l_contiguous is not set to 1,
> + but where it could be set: If a PIE (ET_DYN) binary is loaded by
> + glibc itself (not the kernel), it is always contiguous due to the
> + way the glibc loader works. However, the kernel loader may still
> + create holes in this case, and the code here still uses 0
> + conservatively for the glibc-loaded case, too. */
> + ElfW(Addr) expected_load_address = 0;
>
> /* Scan the program header table for the dynamic section. */
> for (const ElfW(Phdr) *ph = phdr; ph < &phdr[phnum]; ++ph)
> @@ -1207,12 +1223,21 @@ rtld_setup_main_map (struct link_map *main_map)
> if (main_map->l_map_start > mapstart)
> main_map->l_map_start = mapstart;
>
> + if (main_map->l_contiguous && expected_load_address != 0
> + && expected_load_address != mapstart)
> + main_map->l_contiguous = 0;
> +
> /* Also where it ends. */
> 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. */
> + expected_load_address = ((allocend + GLRO(dl_pagesize) - 1)
> + & ~(GLRO(dl_pagesize) - 1));
> }
> break;
>
> --
> 2.33.1
>
LGTM.
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/3] elf: Introduce rtld_setup_main_map
2022-01-03 17:11 ` [PATCH 1/3] elf: Introduce rtld_setup_main_map Florian Weimer
@ 2022-01-16 0:11 ` H.J. Lu
0 siblings, 0 replies; 18+ messages in thread
From: H.J. Lu @ 2022-01-16 0:11 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Mon, Jan 3, 2022 at 9:12 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> This function collects most of the processing needed to initialize
> the link map for the main executable.
> ---
> elf/rtld.c | 303 ++++++++++++++++++++++++++++-------------------------
> 1 file changed, 159 insertions(+), 144 deletions(-)
>
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 24e48bf3fa..ba6e31377d 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1126,17 +1126,172 @@ rtld_chain_load (struct link_map *main_map, char *argv0)
> rtld_soname, pathname, errcode);
> }
>
> +/* Called to complete the initialization of the link map for the main
> + executable. Returns true if there is a PT_INTERP segment. */
> +static bool
> +rtld_setup_main_map (struct link_map *main_map)
> +{
> + /* This have already been filled in right after _dl_new_object, or
> + as part of _dl_map_object. */
> + const ElfW(Phdr) *phdr = main_map->l_phdr;
> + ElfW(Word) phnum = main_map->l_phnum;
> +
> + 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. */
> + ++main_map->l_direct_opencount;
> +
> + /* Scan the program header table for the dynamic section. */
> + for (const ElfW(Phdr) *ph = phdr; ph < &phdr[phnum]; ++ph)
> + switch (ph->p_type)
> + {
> + case PT_PHDR:
> + /* Find out the load address. */
> + main_map->l_addr = (ElfW(Addr)) phdr - ph->p_vaddr;
> + break;
> + case PT_DYNAMIC:
> + /* This tells us where to find the dynamic section,
> + which tells us everything we need to do. */
> + main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
> + main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
> + break;
> + case PT_INTERP:
> + /* This "interpreter segment" was used by the program loader to
> + find the program interpreter, which is this program itself, the
> + dynamic linker. We note what name finds us, so that a future
> + dlopen call or DT_NEEDED entry, for something that wants to link
> + against the dynamic linker as a shared library, will know that
> + the shared object is already loaded. */
> + _dl_rtld_libname.name = ((const char *) main_map->l_addr
> + + ph->p_vaddr);
> + /* _dl_rtld_libname.next = NULL; Already zero. */
> + GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
> +
> + /* Ordinarilly, we would get additional names for the loader from
> + our DT_SONAME. This can't happen if we were actually linked as
> + a static executable (detect this case when we have no DYNAMIC).
> + If so, assume the filename component of the interpreter path to
> + be our SONAME, and add it to our name list. */
> + if (GL(dl_rtld_map).l_ld == NULL)
> + {
> + const char *p = NULL;
> + const char *cp = _dl_rtld_libname.name;
> +
> + /* Find the filename part of the path. */
> + while (*cp != '\0')
> + if (*cp++ == '/')
> + p = cp;
> +
> + if (p != NULL)
> + {
> + _dl_rtld_libname2.name = p;
> + /* _dl_rtld_libname2.next = NULL; Already zero. */
> + _dl_rtld_libname.next = &_dl_rtld_libname2;
> + }
> + }
> +
> + has_interp = true;
> + break;
> + case PT_LOAD:
> + {
> + ElfW(Addr) mapstart;
> + ElfW(Addr) allocend;
> +
> + /* Remember where the main program starts in memory. */
> + mapstart = (main_map->l_addr
> + + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
> + if (main_map->l_map_start > mapstart)
> + main_map->l_map_start = mapstart;
> +
> + /* Also where it ends. */
> + 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;
> + }
> + break;
> +
> + case PT_TLS:
> + if (ph->p_memsz > 0)
> + {
> + /* Note that in the case the dynamic linker we duplicate work
> + here since we read the PT_TLS entry already in
> + _dl_start_final. But the result is repeatable so do not
> + check for this special but unimportant case. */
> + main_map->l_tls_blocksize = ph->p_memsz;
> + main_map->l_tls_align = ph->p_align;
> + if (ph->p_align == 0)
> + main_map->l_tls_firstbyte_offset = 0;
> + else
> + main_map->l_tls_firstbyte_offset = (ph->p_vaddr
> + & (ph->p_align - 1));
> + main_map->l_tls_initimage_size = ph->p_filesz;
> + main_map->l_tls_initimage = (void *) ph->p_vaddr;
> +
> + /* This image gets the ID one. */
> + GL(dl_tls_max_dtv_idx) = main_map->l_tls_modid = 1;
> + }
> + break;
> +
> + case PT_GNU_STACK:
> + GL(dl_stack_flags) = ph->p_flags;
> + break;
> +
> + case PT_GNU_RELRO:
> + main_map->l_relro_addr = ph->p_vaddr;
> + main_map->l_relro_size = ph->p_memsz;
> + break;
> + }
> + /* Process program headers again, but scan them backwards so
> + that PT_NOTE can be skipped if PT_GNU_PROPERTY exits. */
> + for (const ElfW(Phdr) *ph = &phdr[phnum]; ph != phdr; --ph)
> + switch (ph[-1].p_type)
> + {
> + case PT_NOTE:
> + _dl_process_pt_note (main_map, -1, &ph[-1]);
> + break;
> + case PT_GNU_PROPERTY:
> + _dl_process_pt_gnu_property (main_map, -1, &ph[-1]);
> + break;
> + }
> +
> + /* Adjust the address of the TLS initialization image in case
> + the executable is actually an ET_DYN object. */
> + if (main_map->l_tls_initimage != NULL)
> + main_map->l_tls_initimage
> + = (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
> + PT_INTERP. */
> + _dl_rtld_libname.name = GL(dl_rtld_map).l_name;
> + /* _dl_rtld_libname.next = NULL; Already zero. */
> + GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
> + }
> + else
> + assert (GL(dl_rtld_map).l_libname); /* How else did we get here? */
> +
> + return has_interp;
> +}
> +
> static void
> dl_main (const ElfW(Phdr) *phdr,
> ElfW(Word) phnum,
> ElfW(Addr) *user_entry,
> ElfW(auxv_t) *auxv)
> {
> - const ElfW(Phdr) *ph;
> struct link_map *main_map;
> size_t file_size;
> char *file;
> - bool has_interp = false;
> unsigned int i;
> bool prelinked = false;
> bool rtld_is_main = false;
> @@ -1350,7 +1505,7 @@ dl_main (const ElfW(Phdr) *phdr,
> load the program below unless it has a PT_GNU_STACK indicating
> nonexecutable stack is ok. */
>
> - for (ph = phdr; ph < &phdr[phnum]; ++ph)
> + for (const ElfW(Phdr) *ph = phdr; ph < &phdr[phnum]; ++ph)
> if (ph->p_type == PT_GNU_STACK)
> {
> GL(dl_stack_flags) = ph->p_flags;
> @@ -1469,147 +1624,7 @@ dl_main (const ElfW(Phdr) *phdr,
> information for the program. */
> }
>
> - 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. */
> - ++main_map->l_direct_opencount;
> -
> - /* Scan the program header table for the dynamic section. */
> - for (ph = phdr; ph < &phdr[phnum]; ++ph)
> - switch (ph->p_type)
> - {
> - case PT_PHDR:
> - /* Find out the load address. */
> - main_map->l_addr = (ElfW(Addr)) phdr - ph->p_vaddr;
> - break;
> - case PT_DYNAMIC:
> - /* This tells us where to find the dynamic section,
> - which tells us everything we need to do. */
> - main_map->l_ld = (void *) main_map->l_addr + ph->p_vaddr;
> - main_map->l_ld_readonly = (ph->p_flags & PF_W) == 0;
> - break;
> - case PT_INTERP:
> - /* This "interpreter segment" was used by the program loader to
> - find the program interpreter, which is this program itself, the
> - dynamic linker. We note what name finds us, so that a future
> - dlopen call or DT_NEEDED entry, for something that wants to link
> - against the dynamic linker as a shared library, will know that
> - the shared object is already loaded. */
> - _dl_rtld_libname.name = ((const char *) main_map->l_addr
> - + ph->p_vaddr);
> - /* _dl_rtld_libname.next = NULL; Already zero. */
> - GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
> -
> - /* Ordinarilly, we would get additional names for the loader from
> - our DT_SONAME. This can't happen if we were actually linked as
> - a static executable (detect this case when we have no DYNAMIC).
> - If so, assume the filename component of the interpreter path to
> - be our SONAME, and add it to our name list. */
> - if (GL(dl_rtld_map).l_ld == NULL)
> - {
> - const char *p = NULL;
> - const char *cp = _dl_rtld_libname.name;
> -
> - /* Find the filename part of the path. */
> - while (*cp != '\0')
> - if (*cp++ == '/')
> - p = cp;
> -
> - if (p != NULL)
> - {
> - _dl_rtld_libname2.name = p;
> - /* _dl_rtld_libname2.next = NULL; Already zero. */
> - _dl_rtld_libname.next = &_dl_rtld_libname2;
> - }
> - }
> -
> - has_interp = true;
> - break;
> - case PT_LOAD:
> - {
> - ElfW(Addr) mapstart;
> - ElfW(Addr) allocend;
> -
> - /* Remember where the main program starts in memory. */
> - mapstart = (main_map->l_addr
> - + (ph->p_vaddr & ~(GLRO(dl_pagesize) - 1)));
> - if (main_map->l_map_start > mapstart)
> - main_map->l_map_start = mapstart;
> -
> - /* Also where it ends. */
> - 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;
> - }
> - break;
> -
> - case PT_TLS:
> - if (ph->p_memsz > 0)
> - {
> - /* Note that in the case the dynamic linker we duplicate work
> - here since we read the PT_TLS entry already in
> - _dl_start_final. But the result is repeatable so do not
> - check for this special but unimportant case. */
> - main_map->l_tls_blocksize = ph->p_memsz;
> - main_map->l_tls_align = ph->p_align;
> - if (ph->p_align == 0)
> - main_map->l_tls_firstbyte_offset = 0;
> - else
> - main_map->l_tls_firstbyte_offset = (ph->p_vaddr
> - & (ph->p_align - 1));
> - main_map->l_tls_initimage_size = ph->p_filesz;
> - main_map->l_tls_initimage = (void *) ph->p_vaddr;
> -
> - /* This image gets the ID one. */
> - GL(dl_tls_max_dtv_idx) = main_map->l_tls_modid = 1;
> - }
> - break;
> -
> - case PT_GNU_STACK:
> - GL(dl_stack_flags) = ph->p_flags;
> - break;
> -
> - case PT_GNU_RELRO:
> - main_map->l_relro_addr = ph->p_vaddr;
> - main_map->l_relro_size = ph->p_memsz;
> - break;
> - }
> - /* Process program headers again, but scan them backwards so
> - that PT_NOTE can be skipped if PT_GNU_PROPERTY exits. */
> - for (ph = &phdr[phnum]; ph != phdr; --ph)
> - switch (ph[-1].p_type)
> - {
> - case PT_NOTE:
> - _dl_process_pt_note (main_map, -1, &ph[-1]);
> - break;
> - case PT_GNU_PROPERTY:
> - _dl_process_pt_gnu_property (main_map, -1, &ph[-1]);
> - break;
> - }
> -
> - /* Adjust the address of the TLS initialization image in case
> - the executable is actually an ET_DYN object. */
> - if (main_map->l_tls_initimage != NULL)
> - main_map->l_tls_initimage
> - = (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
> - PT_INTERP. */
> - _dl_rtld_libname.name = GL(dl_rtld_map).l_name;
> - /* _dl_rtld_libname.next = NULL; Already zero. */
> - GL(dl_rtld_map).l_libname = &_dl_rtld_libname;
> - }
> - else
> - assert (GL(dl_rtld_map).l_libname); /* How else did we get here? */
> + bool has_interp = rtld_setup_main_map (main_map);
>
> /* If the current libname is different from the SONAME, add the
> latter as well. */
> --
> 2.33.1
>
LGTM.
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732)
2022-01-03 17:11 ` [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732) Florian Weimer
2022-01-14 15:06 ` H.J. Lu
@ 2022-01-16 14:05 ` H.J. Lu
1 sibling, 0 replies; 18+ messages in thread
From: H.J. Lu @ 2022-01-16 14:05 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Mon, Jan 3, 2022 at 9:13 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> ---
> elf/tst-dl_find_object.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/elf/tst-dl_find_object.c b/elf/tst-dl_find_object.c
> index 21cdc0f848..2ad1924088 100644
> --- a/elf/tst-dl_find_object.c
> +++ b/elf/tst-dl_find_object.c
> @@ -71,19 +71,24 @@ check (void *address,
> __FILE__, line, address,
> actual.dlfo_flags, expected->dlfo_flags);
> }
> - if (actual.dlfo_flags != expected->dlfo_flags)
> + if (expected->dlfo_link_map->l_contiguous)
> {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_start, expected->dlfo_map_start);
> - }
> - if (actual.dlfo_map_end != expected->dlfo_map_end)
> - {
> - support_record_failure ();
> - printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> - __FILE__, line,
> - address, actual.dlfo_map_end, expected->dlfo_map_end);
> + /* If the mappings are not contiguous, the actual and execpted
> + mappings may differ, so this subtest will not work. */
> + if (actual.dlfo_flags != expected->dlfo_flags)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map start is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_start, expected->dlfo_map_start);
> + }
> + if (actual.dlfo_map_end != expected->dlfo_map_end)
> + {
> + support_record_failure ();
> + printf ("%s:%d: error: %p: map end is %p, expected %p\n",
> + __FILE__, line,
> + address, actual.dlfo_map_end, expected->dlfo_map_end);
> + }
> }
> if (actual.dlfo_link_map != expected->dlfo_link_map)
> {
> --
> 2.33.1
>
LGTM.
Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-01-16 14:06 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03 17:11 [PATCH 0/3] Fix elf/tst-dl_find_objects with --enable-hardcoded-path-in-tests Florian Weimer
2022-01-03 17:11 ` [PATCH 1/3] elf: Introduce rtld_setup_main_map Florian Weimer
2022-01-16 0:11 ` H.J. Lu
2022-01-03 17:11 ` [PATCH 2/3] elf: Set l_contiguous to 1 for the main map in more cases Florian Weimer
2022-01-16 0:10 ` H.J. Lu
2022-01-03 17:11 ` [PATCH 3/3] elf/tst-dl_find_object: Disable subtests for non-contiguous maps (bug 28732) Florian Weimer
2022-01-14 15:06 ` H.J. Lu
2022-01-14 15:09 ` H.J. Lu
2022-01-14 15:10 ` Florian Weimer
2022-01-14 15:19 ` H.J. Lu
2022-01-14 15:39 ` Florian Weimer
2022-01-14 15:47 ` H.J. Lu
2022-01-14 15:51 ` Florian Weimer
2022-01-14 15:54 ` H.J. Lu
2022-01-14 15:56 ` Florian Weimer
2022-01-14 16:06 ` H.J. Lu
2022-01-14 16:12 ` Florian Weimer
2022-01-16 14:05 ` H.J. Lu
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).