* [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
* 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
* [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
* 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
* [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 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).