[commit] [patch] Fix run-unstrip-n.sh regression on CentOS-5

Jan Kratochvil jan.kratochvil at redhat.com
Tue Jul 23 14:32:16 UTC 2013


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 at 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 at 0x8048178 . - [exe]
    +0xf75e9000+0x1a000 29a103420abe341e92072fb14274e250e4072148 at 0xf75e9164 - - libpthread.so.0
    +0xf7603000+0x1b0000 0b9bf374699e141e5dfc14757ff42b8c2373b4de at 0xf7603184 - - libc.so.6
    +0xf77b3000+0x9000 c6c5b5e35ab9589d4762ac85b4bd56b1b2720e37 at 0xf77b3164 - - librt.so.1
     0xf77d6000+0x1000 676560b1b765cde9c2e53f134f4ee354ea894747 at 0xf77d6210 . - linux-gate.so.1
    +0xf77d7000+0x21000 6d2cb32650054f1c176d01d48713a4a5e5e84c1a at 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 at 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 at 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 at 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 at 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 at 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 at redhat.com>
+
+	* run-unstrip-n.sh (test-core.*): Ignore libc.so.6 entry and order of
+	the entries.
+
 2013-07-02  Mark Wielaard  <mjw at 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 at 0x7fff1596c340 . - linux-vdso.so.1


More information about the elfutils-devel mailing list