This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
[commit] [patch] Fix run-unstrip-n.sh regression on CentOS-5
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 23 Jul 2013 16:32:16 +0200
- Subject: [commit] [patch] Fix run-unstrip-n.sh regression on CentOS-5
On Tue, 23 Jul 2013 00:13:07 +0200, Mark Wielaard wrote:
> Looks OK to me, but I haven't tried it.
Checked in.
Thanks,
Jan
commit 596d430f23f85f3cd019bd0ac560ecd5371fc7e0
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date: Tue Jul 23 16:30:01 2013 +0200
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(a)0x8048178 . - [exe]
+0xf75e9000+0x1a000 29a103420abe341e92072fb14274e250e4072148(a)0xf75e9164 - - libpthread.so.0
+0xf7603000+0x1b0000 0b9bf374699e141e5dfc14757ff42b8c2373b4de(a)0xf7603184 - - libc.so.6
+0xf77b3000+0x9000 c6c5b5e35ab9589d4762ac85b4bd56b1b2720e37(a)0xf77b3164 - - librt.so.1
0xf77d6000+0x1000 676560b1b765cde9c2e53f134f4ee354ea894747(a)0xf77d6210 . - linux-gate.so.1
+0xf77d7000+0x21000 6d2cb32650054f1c176d01d48713a4a5e5e84c1a(a)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 <jan.kratochvil@redhat.com>
* 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 <jan.kratochvil@redhat.com>
* run-unstrip-n.sh (test-core.*): Ignore libc.so.6 entry and order of
the entries.
Signed-off-by: Jan Kratochvil <jan.kratochvil@redhat.com>
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index af665f4..8ab6b5b 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,24 @@
+2013-07-23 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * 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 <jan.kratochvil@redhat.com>
* libdwflP.h (__libdwfl_find_elf_build_id): Add internal_function.
diff --git a/libdwfl/core-file.c b/libdwfl/core-file.c
index d5e340c..7207591 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 20d04da..3a4ae2e 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 d454ccb..41487bf 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -37,6 +37,7 @@
#include <sys/param.h>
#include <alloca.h>
#include <endian.h>
+#include <unistd.h>
/* 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 1ca4763..5437b37 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 fecf616..65e1c3a 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 a080a73..ea1f2de 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2013-07-23 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * run-unstrip-n.sh (test-core.*): Ignore libc.so.6 entry and order of
+ the entries.
+
2013-07-02 Mark Wielaard <mjw@redhat.com>
* Makefile.am (EXTRA_DIST): Fix typo, forgot extension in
diff --git a/tests/run-unstrip-n.sh b/tests/run-unstrip-n.sh
index 6ede4c7..12c3822 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 - <<EOF
0x400000+0x202038 - test-core.exec - test-core.exec
0x7f67f2aaf000+0x202000 - . - test-core-lib.so
0x7fff1596c000+0x1000 a9cf37f53897b5468ee018655760be61b8633d3c(a)0x7fff1596c340 . - linux-vdso.so.1