]> git.ipfire.org Git - thirdparty/elfutils.git/commitdiff
Fix false match of non-build-id disk library to build-id memory library.
authorJan Kratochvil <jan.kratochvil@redhat.com>
Tue, 23 Jul 2013 14:30:01 +0000 (16:30 +0200)
committerJan Kratochvil <jan.kratochvil@redhat.com>
Tue, 23 Jul 2013 14:30:01 +0000 (16:30 +0200)
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  <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>
libdwfl/ChangeLog
libdwfl/core-file.c
libdwfl/dwfl_report_elf.c
libdwfl/dwfl_segment_report_module.c
libdwfl/libdwflP.h
libdwfl/link_map.c
tests/ChangeLog
tests/run-unstrip-n.sh

index af665f469446421b70be319d0579197648db1560..8ab6b5ba999695c8cb48ce9b786704e759b2ebce 100644 (file)
@@ -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.
index d5e340cbe5312dd07bed671a3caf3fd93e583be4..72075913ce475571c57e4067998e5b91b0ab6e89 100644 (file)
@@ -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.
index 20d04da1653f57c4bd65c4ff1a7e8c162bbba47b..3a4ae2ef180f3585d17ac38e3bf6b118b7b97ffc 100644 (file)
    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
        {
index d454ccb50189a2fc138438ff4ebc4b00ebdf577b..41487bf007a01aa0c2e390fca8f83f35fe01794c 100644 (file)
@@ -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.  */
index 1ca47639775cc9f2bfa70857bbdcf7e35c1bff87..5437b37fc35842c318e62db130285aa7a2b1fc8e 100644 (file)
@@ -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,
index fecf616d2c5dc84cdc4be84d0ec1b4522d75fb45..65e1c3a6ed51a32d3bc52c0785a9ec7e2397a1b4 100644 (file)
@@ -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)
        {
index a080a737132b5f429c5350d8452610d1459a47bb..ea1f2dedaf25c1c9994c83f0167c5dcb764ef8e5 100644 (file)
@@ -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
index 6ede4c7e6bfb050b9dbed723066c74d0749c6b0a..12c38229b74213a4ad3ccae932823efe1a8c23e6 100755 (executable)
@@ -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@0x7fff1596c340 . - linux-vdso.so.1