From: Jan Kratochvil Date: Tue, 23 Jul 2013 14:30:01 +0000 (+0200) Subject: Fix false match of non-build-id disk library to build-id memory library. X-Git-Tag: elfutils-0.156~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=596d430f23f85f3cd019bd0ac560ecd5371fc7e0;p=thirdparty%2Felfutils.git Fix false match of non-build-id disk library to build-id memory library. this patch: Use DT_DEBUG library search first. 8ff862960efb648cdff647d7fad1be5acffe9b11 [patch 2/2] Fix loading core files without build-ids https://lists.fedorahosted.org/pipermail/elfutils-devel/2013-April/003031.html [patch 2/2 v2] Fix loading core files without build-ids https://lists.fedorahosted.org/pipermail/elfutils-devel/2013-May/003065.html has PASS->FAIL regression on CentOS-5 for run-unstrip-n.sh: -actual on CentOS-5 +expected by testcase -0xf77b3000+0x822c - /lib/librt.so.1 - librt.so.1 -0xf7603000+0x15c5c4 - /lib/libc.so.6 - libc.so.6 -0xf75e9000+0x191e4 - /lib/libpthread.so.0 - libpthread.so.0 -0xf77d7000+0x1c670 - /lib/ld-linux.so.2 - ld-linux.so.2 0x8048000+0x2000 f1c600bc36cb91bf01f9a63a634ecb79aa4c3199@0x8048178 . - [exe] +0xf75e9000+0x1a000 29a103420abe341e92072fb14274e250e4072148@0xf75e9164 - - libpthread.so.0 +0xf7603000+0x1b0000 0b9bf374699e141e5dfc14757ff42b8c2373b4de@0xf7603184 - - libc.so.6 +0xf77b3000+0x9000 c6c5b5e35ab9589d4762ac85b4bd56b1b2720e37@0xf77b3164 - - librt.so.1 0xf77d6000+0x1000 676560b1b765cde9c2e53f134f4ee354ea894747@0xf77d6210 . - linux-gate.so.1 +0xf77d7000+0x21000 6d2cb32650054f1c176d01d48713a4a5e5e84c1a@0xf77d7124 - - ld-linux.so.2 Therefore elfutils now incorrectly matches on-disk file without build-id to an in-core (in-memory) file with build-id. In fact due to its known FIXME: This verification gives false positive if in-core ELF had build-id but on-disk ELF does not have any. But we cannot reliably find ELF header and/or the ELF build id just from the link map (and checking core segments is also not reliable). */ So it probably should not be so ignorable as I did, one may want to analyze build-id core files on CentOS-5, not sure. In fact it can be fixed, when we find in dwfl_segment_report_module a module with build-id with conflicts in its address range with existing non-build-id dwfl_link_map_report module we should prefer the build-id module instead. The problem is that once Dwfl_Module is added to Dwfl it cannot be easily removed. Originally elfutils called dwfl_segment_report_module first and then dwfl_link_map_report. Currently the order is dwfl_link_map_report and then dwfl_segment_report_module only for modules missing from dwfl_link_map_report. Patch below unfortunately needs bidirectional negotiation between the two functions, therefore dwfl_link_map_report now no longer adds Dwfl_Modules to Dwfl but it only stores information about them to r_debug_info_module. This information is filtered then by dwfl_segment_report_module and only filtered r_debug_info_module entries get finally added to Dwfl (in dwfl_core_file_report). NT_FILE would make all this magic easy but it is true that on CentOS-5 it definitely does not exist. libdwfl/ 2013-07-23 Jan Kratochvil * core-file.c (clear_r_debug_info): Close also ELF and FD. (dwfl_core_file_report): Call __libdwfl_report_elf for R_DEBUG_INFO.MODULE. * dwfl_report_elf.c (__libdwfl_elf_address_range): New function from code of ... (__libdwfl_report_elf): ... this function. Call it. * dwfl_segment_report_module.c: Include unistd.h. (dwfl_segment_report_module): Use basename for MODULE->NAME. Clear MODULE if it has no build-id and we have segment with build-id. Ignore this segment only if MODULE still contains valid ELF. * libdwflP.h (__libdwfl_elf_address_range): New declaration. (struct r_debug_info_module): New fields fd, elf, l_addr, start, end and disk_file_has_build_id. (dwfl_link_map_report): Extend the comment. * link_map.c (report_r_debug): Extend the comment. Always fill in new r_debug_info_module. Initialize also the new r_debug_info_module fields. Remove one FIXME comment. Call __libdwfl_elf_address_range instead of __libdwfl_report_elf when R_DEBUG_INFO is not NULL. tests/ 2013-07-23 Jan Kratochvil * run-unstrip-n.sh (test-core.*): Ignore libc.so.6 entry and order of the entries. Signed-off-by: Jan Kratochvil --- diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index af665f469..8ab6b5ba9 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,24 @@ +2013-07-23 Jan Kratochvil + + * core-file.c (clear_r_debug_info): Close also ELF and FD. + (dwfl_core_file_report): Call __libdwfl_report_elf for + R_DEBUG_INFO.MODULE. + * dwfl_report_elf.c (__libdwfl_elf_address_range): New function from + code of ... + (__libdwfl_report_elf): ... this function. Call it. + * dwfl_segment_report_module.c: Include unistd.h. + (dwfl_segment_report_module): Use basename for MODULE->NAME. + Clear MODULE if it has no build-id and we have segment with build-id. + Ignore this segment only if MODULE still contains valid ELF. + * libdwflP.h (__libdwfl_elf_address_range): New declaration. + (struct r_debug_info_module): New fields fd, elf, l_addr, start, end + and disk_file_has_build_id. + (dwfl_link_map_report): Extend the comment. + * link_map.c (report_r_debug): Extend the comment. Always fill in new + r_debug_info_module. Initialize also the new r_debug_info_module + fields. Remove one FIXME comment. Call __libdwfl_elf_address_range + instead of __libdwfl_report_elf when R_DEBUG_INFO is not NULL. + 2013-07-19 Jan Kratochvil * libdwflP.h (__libdwfl_find_elf_build_id): Add internal_function. diff --git a/libdwfl/core-file.c b/libdwfl/core-file.c index d5e340cbe..72075913c 100644 --- a/libdwfl/core-file.c +++ b/libdwfl/core-file.c @@ -390,6 +390,9 @@ clear_r_debug_info (struct r_debug_info *r_debug_info) { struct r_debug_info_module *module = r_debug_info->module; r_debug_info->module = module->next; + elf_end (module->elf); + if (module->fd != -1) + close (module->fd); free (module); } } @@ -476,6 +479,43 @@ dwfl_core_file_report (Dwfl *dwfl, Elf *elf) } while (ndx < (int) phnum); + /* Now report the modules from dwfl_link_map_report which were not filtered + out by dwfl_segment_report_module. */ + + Dwfl_Module **lastmodp = &dwfl->modulelist; + while (*lastmodp != NULL) + lastmodp = &(*lastmodp)->next; + for (struct r_debug_info_module *module = r_debug_info.module; + module != NULL; module = module->next) + if (module->elf != NULL) + { + Dwfl_Module *mod; + mod = __libdwfl_report_elf (dwfl, basename (module->name), module->name, + module->fd, module->elf, module->l_addr, + true, true); + if (mod == NULL) + continue; + module->elf = NULL; + module->fd = -1; + /* Move this module to the end of the list, so that we end + up with a list in the same order as the link_map chain. */ + if (mod->next != NULL) + { + if (*lastmodp != mod) + { + lastmodp = &dwfl->modulelist; + while (*lastmodp != mod) + lastmodp = &(*lastmodp)->next; + } + *lastmodp = mod->next; + mod->next = NULL; + while (*lastmodp != NULL) + lastmodp = &(*lastmodp)->next; + *lastmodp = mod; + } + lastmodp = &mod->next; + } + clear_r_debug_info (&r_debug_info); /* We return the number of modules we found if we found any. diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c index 20d04da16..3a4ae2ef1 100644 --- a/libdwfl/dwfl_report_elf.c +++ b/libdwfl/dwfl_report_elf.c @@ -38,18 +38,20 @@ rejiggering (see below). */ #define REL_MIN_ALIGN ((GElf_Xword) 0x100) -Dwfl_Module * +bool internal_function -__libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, - int fd, Elf *elf, GElf_Addr base, bool add_p_vaddr, - bool sanity) +__libdwfl_elf_address_range (Elf *elf, GElf_Addr base, bool add_p_vaddr, + bool sanity, GElf_Addr *vaddrp, + GElf_Addr *address_syncp, GElf_Addr *startp, + GElf_Addr *endp, GElf_Addr *biasp, + GElf_Half *e_typep) { GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr (elf, &ehdr_mem); if (ehdr == NULL) { elf_error: __libdwfl_seterrno (DWFL_E_LIBELF); - return NULL; + return false; } GElf_Addr vaddr = 0; @@ -213,11 +215,37 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, if (end == 0 && sanity) { __libdwfl_seterrno (DWFL_E_NO_PHDR); - return NULL; + return false; } break; } + if (vaddrp) + *vaddrp = vaddr; + if (address_syncp) + *address_syncp = address_sync; + if (startp) + *startp = start; + if (endp) + *endp = end; + if (biasp) + *biasp = bias; + if (e_typep) + *e_typep = ehdr->e_type; + return true; +} +Dwfl_Module * +internal_function +__libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, + int fd, Elf *elf, GElf_Addr base, bool add_p_vaddr, + bool sanity) +{ + GElf_Addr vaddr, address_sync, start, end, bias; + GElf_Half e_type; + if (! __libdwfl_elf_address_range (elf, base, add_p_vaddr, sanity, &vaddr, + &address_sync, &start, &end, &bias, + &e_type)) + return NULL; Dwfl_Module *m = INTUSE(dwfl_report_module) (dwfl, name, start, end); if (m != NULL) { @@ -242,7 +270,7 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const char *file_name, m->main.vaddr = vaddr; m->main.address_sync = address_sync; m->main_bias = bias; - m->e_type = ehdr->e_type; + m->e_type = e_type; } else { diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c index d454ccb50..41487bf00 100644 --- a/libdwfl/dwfl_segment_report_module.c +++ b/libdwfl/dwfl_segment_report_module.c @@ -37,6 +37,7 @@ #include #include #include +#include /* A good size for the initial read from memory, if it's not too costly. @@ -462,7 +463,7 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, bias += fixup; if (module->name[0] != '\0') { - name = module->name; + name = basename (module->name); name_is_final = true; } break; @@ -471,9 +472,26 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, /* Ignore this found module if it would conflict in address space with any already existing module of DWFL. */ - for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL; mod = mod->next) - if (module_end > mod->low_addr && module_start < mod->high_addr) - return finish (); + if (r_debug_info != NULL) + for (struct r_debug_info_module *module = r_debug_info->module; + module != NULL; module = module->next) + if ((module_end > module->start && module_start < module->end) + || (module_start <= module->l_ld && module->l_ld < module_end)) + { + if (! module->disk_file_has_build_id && build_id_len > 0) + { + if (module->elf != NULL) + { + elf_end (module->elf); + close (module->fd); + module->elf = NULL; + module->fd = -1; + } + break; + } + else if (module->elf != NULL) + return finish (); + } /* Our return value now says to skip the segments contained within the module. */ diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h index 1ca476397..5437b37fc 100644 --- a/libdwfl/libdwflP.h +++ b/libdwfl/libdwflP.h @@ -389,6 +389,16 @@ extern uint32_t __libdwfl_crc32 (uint32_t crc, unsigned char *buf, size_t len) extern int __libdwfl_crc32_file (int fd, uint32_t *resp) attribute_hidden; +/* Given ELF and some parameters return TRUE if the *P return value parameters + have been successfully filled in. Any of the *P parameters can be NULL. */ +extern bool __libdwfl_elf_address_range (Elf *elf, GElf_Addr base, + bool add_p_vaddr, bool sanity, + GElf_Addr *vaddrp, + GElf_Addr *address_syncp, + GElf_Addr *startp, GElf_Addr *endp, + GElf_Addr *biasp, GElf_Half *e_typep); + internal_function; + /* Meat of dwfl_report_elf, given elf_begin just called. Consumes ELF on success, not on failure. */ extern Dwfl_Module *__libdwfl_report_elf (Dwfl *dwfl, const char *name, @@ -454,7 +464,13 @@ typedef bool Dwfl_Module_Callback (Dwfl_Module *mod, void **userdata, struct r_debug_info_module { struct r_debug_info_module *next; - GElf_Addr l_ld; + /* FD is -1 iff ELF is NULL. */ + int fd; + Elf *elf; + GElf_Addr l_addr, l_ld; + /* START and END are both zero if not valid. */ + GElf_Addr start, end; + bool disk_file_has_build_id; char name[0]; }; @@ -488,6 +504,8 @@ extern int dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, Fill in R_DEBUG_INFO if it is not NULL. It should be cleared by the caller, this function does not touch fields it does not need to modify. + If R_DEBUG_INFO is not NULL then no modules get added to DWFL, caller + has to add them from filled in R_DEBUG_INFO. Returns the number of modules found, or -1 for errors. */ extern int dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t auxv_size, diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c index fecf616d2..65e1c3a6e 100644 --- a/libdwfl/link_map.c +++ b/libdwfl/link_map.c @@ -224,7 +224,9 @@ addrsize (uint_fast8_t elfclass) /* Report a module for each struct link_map in the linked list at r_map in the struct r_debug at R_DEBUG_VADDR. For r_debug_info description - see dwfl_link_map_report in libdwflP.h. + see dwfl_link_map_report in libdwflP.h. If R_DEBUG_INFO is not NULL then no + modules get added to DWFL, caller has to add them from filled in + R_DEBUG_INFO. For each link_map entry, if an existing module resides at its address, this just modifies that module's name and suggested file name. If @@ -355,6 +357,28 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata, if (iterations == 1 && dwfl->executable_for_core != NULL) name = dwfl->executable_for_core; + struct r_debug_info_module *r_debug_info_module = NULL; + if (r_debug_info != NULL) + { + /* Save link map information about valid shared library (or + executable) which has not been found on disk. */ + const char *name1 = name == NULL ? "" : name; + r_debug_info_module = malloc (sizeof (*r_debug_info_module) + + strlen (name1) + 1); + if (r_debug_info_module == NULL) + return release_buffer (result); + r_debug_info_module->fd = -1; + r_debug_info_module->elf = NULL; + r_debug_info_module->l_addr = l_addr; + r_debug_info_module->l_ld = l_ld; + r_debug_info_module->start = 0; + r_debug_info_module->end = 0; + r_debug_info_module->disk_file_has_build_id = false; + strcpy (r_debug_info_module->name, name1); + r_debug_info_module->next = r_debug_info->module; + r_debug_info->module = r_debug_info_module; + } + Dwfl_Module *mod = NULL; if (name != NULL) { @@ -374,19 +398,15 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata, /* FIXME: Bias L_ADDR should be computed from the prelink state in memory (when the file got loaded), not against - the current on-disk file state as is computed below. - - This verification gives false positive if in-core ELF had - build-id but on-disk ELF does not have any. But we cannot - reliably find ELF header and/or the ELF build id just from - the link map (and checking core segments is also not - reliable). */ + the current on-disk file state as is computed below. */ if (__libdwfl_find_elf_build_id (NULL, elf, &build_id_bits, &build_id_elfaddr, &build_id_len) > 0 && build_id_elfaddr != 0) { + if (r_debug_info_module != NULL) + r_debug_info_module->disk_file_has_build_id = true; GElf_Addr build_id_vaddr = build_id_elfaddr + l_addr; release_buffer (0); int segndx = INTUSE(dwfl_addrsegment) (dwfl, @@ -410,31 +430,38 @@ report_r_debug (uint_fast8_t elfclass, uint_fast8_t elfdata, } if (valid) - // XXX hook for sysroot - mod = __libdwfl_report_elf (dwfl, basename (name), name, - fd, elf, l_addr, true, true); - if (mod == NULL) { - elf_end (elf); - close (fd); + if (r_debug_info_module == NULL) + { + // XXX hook for sysroot + mod = __libdwfl_report_elf (dwfl, basename (name), + name, fd, elf, l_addr, + true, true); + if (mod != NULL) + { + elf = NULL; + fd = -1; + } + } + else if (__libdwfl_elf_address_range (elf, l_addr, true, + true, NULL, NULL, + &r_debug_info_module->start, + &r_debug_info_module->end, + NULL, NULL)) + { + r_debug_info_module->elf = elf; + r_debug_info_module->fd = fd; + elf = NULL; + fd = -1; + } } + if (elf != NULL) + elf_end (elf); + if (fd != -1) + close (fd); } } } - if (r_debug_info != NULL && mod == NULL) - { - /* Save link map information about valid shared library (or - executable) which has not been found on disk. */ - const char *base = name == NULL ? "" : basename (name); - struct r_debug_info_module *module; - module = malloc (sizeof (*module) + strlen (base) + 1); - if (module == NULL) - return release_buffer (result); - module->l_ld = l_ld; - strcpy (module->name, base); - module->next = r_debug_info->module; - r_debug_info->module = module; - } if (mod != NULL) { diff --git a/tests/ChangeLog b/tests/ChangeLog index a080a7371..ea1f2deda 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2013-07-23 Jan Kratochvil + + * run-unstrip-n.sh (test-core.*): Ignore libc.so.6 entry and order of + the entries. + 2013-07-02 Mark Wielaard * Makefile.am (EXTRA_DIST): Fix typo, forgot extension in diff --git a/tests/run-unstrip-n.sh b/tests/run-unstrip-n.sh index 6ede4c7e6..12c38229b 100755 --- a/tests/run-unstrip-n.sh +++ b/tests/run-unstrip-n.sh @@ -60,7 +60,12 @@ EOF # 0x03a000 0x00007fff1596c000 linux-vdso.so.1 testfiles test-core.core test-core.exec rm -f test-core-lib.so -testrun_compare ${abs_top_builddir}/src/unstrip -n -e test-core.exec --core=test-core.core <<\EOF +outfile=test-core.out +testrun_out $outfile ${abs_top_builddir}/src/unstrip -n -e test-core.exec --core=test-core.core +outfile2=test-core.out2 +remove_files="$remove_files $outfile2" +grep -v libc.so.6 $outfile | sort >$outfile2 +diff -u $outfile2 - <