This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Wed, 24 Jul 2013 23:32:52 +0200, Mark Wielaard wrote:
> Please do include the explanation in the commit message (as you done with
> the last commit) so we can easily find it back in the future.

While I did include the text like you do I do not find it so useful.

When chasing regressions in GDB I find most important to find the whole mail
thread where one can find also other opinions on the subject.  If each patch
worked according to its description there would be no regressions. :-)

Just referencing a mail via Message-ID is not convenient for someone not
having the UNIX mbox history (which is downloadable for elfutils but commonly
not for other projects - like GDB).  OTOH pipermail URL is not convenient for
someone already having the local UNIX mbox.  So far I include both in GDB
mails (but none in GDB commits).


> This looks fine to me.

Checked in.


Thanks,
Jan


commit 10edf47b87e0b15459d8f74357e03c9440e3dcf3
Author: Jan Kratochvil <jan.kratochvil@redhat.com>
Date:   Thu Jul 25 11:05:35 2013 +0200

    Fix run-unstrip-n.sh regression on CentOS-5 ppc.
    
    last patch was a bit more heuristic than needed which was found on RHEL-5 ppc
    (32-bit):
    
    FAIL: run-unstrip-n.sh (exit: 1)
    ================================
    --- unstrip.out 2013-07-23 23:23:49.000000000 +0200
    +++ -   2013-07-23 23:23:49.434052534 +0200
    @@ -4,4 +4,3 @@
     0xfdf0000+0x1c0000 edf3dd232e09d01b90683889bd16b9406c52d4de(a)0xfdf0184 - - libc.so.6
     0xffb0000+0x50000 edec437a85026a1cf8cda94003706202733130c1(a)0xffb0124 - - ld.so.1
     0x10000000+0x20000 979b7a26747cc09bd84a42b311b5288c704baea5(a)0x10000174 . - [exe]
    -0xf880000+0x201d4 - /lib/librt.so.1 /usr/lib/debug/lib/librt-2.5.so.debug librt.so.1
    
    Therefore the new code generated this excessive line:
    	0xf880000+0x201d4 - /lib/librt.so.1 /usr/lib/debug/lib/librt-2.5.so.debug librt.so.1
    
    The first part of debug dump is from DT_DEBUG, second part is from segments:
    
    start=0xf880000 end=0xf8a01d4 l_ld=0xfd6fe20 name=/lib/librt.so.1
    start=0xfc60000 end=0xfe031e4 l_ld=0xff9e270 name=/lib/libc.so.6
    start=0xfe10000 end=0xfe421dc l_ld=0xfddfd98 name=/lib/libpthread.so.0
    start=0xffb0000 end=0xfff0668 l_ld=0xffef9ac name=/lib/ld.so.1
    module_start=0x100000 module_end=0x110000 dyn_vaddr=0x100ee4
    module_start=0xfd50000 module_end=0xfd80000 dyn_vaddr=0xfd6fe20 /lib/librt.so.1
    module_start=0xfdb0000 module_end=0xfdf0000 dyn_vaddr=0xfddfd98 /lib/libpthread.so.0
    module_start=0xfdf0000 module_end=0xffb0000 dyn_vaddr=0xff9e270 /lib/libc.so.6
    module_start=0xffb0000 module_end=0x10000000 dyn_vaddr=0xffef9ac /lib/ld.so.1
    module_start=0x10000000 module_end=0x10020000 dyn_vaddr=0x10010850
    
    When comparing conflicts for (found in segments)
    	module_start=0xfd50000 module_end=0xfd80000 dyn_vaddr=0xfd6fe20 /lib/librt.so.1
    
    the code found this line conflicts (and discarded it):
    	start=0xfc60000 end=0xfe031e4 l_ld=0xff9e270 name=/lib/libc.so.6
    
    but it did not discard also conflicting:
    	start=0xf880000 end=0xf8a01d4 l_ld=0xfd6fe20 name=/lib/librt.so.1
    
    So I have changed/improved the algorithm - L_LD can be IMO compared exactly
    but otherwise the ranges should be compared for every module, not just the
    first one.
    
    Again I am not much happy from this code, it should be using NT_FILE instead,
    but when we keep compatibility with old OSes elfutils should not regress
    there.
    
    libdwfl/
    2013-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
    
    	* dwfl_segment_report_module.c (dwfl_segment_report_module): Check for
    	conflicts all the modules, not just the first one.  Compare L_LD if it
    	is equal, not if it is in a module address range.
    
    Signed-off-by: Jan Kratochvil <jan.kratochvil@redhat.com>

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 77faa85..88bdfc6 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,9 @@
+2013-07-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* dwfl_segment_report_module.c (dwfl_segment_report_module): Check for
+	conflicts all the modules, not just the first one.  Compare L_LD if it
+	is equal, not if it is in a module address range.
+
 2013-07-23  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* libdwflP.h (__libdwfl_elf_address_range): Add internal_function.
diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c
index 41487bf..97f4a1a 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -470,28 +470,37 @@ 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.  */
   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 ();
-	}
+    {
+      bool skip_this_module = false;
+      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)
+	    || dyn_vaddr == module->l_ld)
+	  {
+	    if (! module->disk_file_has_build_id && build_id_len > 0)
+	      {
+		/* Module found in segments with build-id is more reliable
+		   than a module found via DT_DEBUG on disk without any
+		   build-id.   */
+		if (module->elf != NULL)
+		  {
+		    elf_end (module->elf);
+		    close (module->fd);
+		    module->elf = NULL;
+		    module->fd = -1;
+		  }
+	      }
+	    else if (module->elf != NULL)
+	      {
+		/* Ignore this found module if it would conflict in address
+		   space with any already existing module of DWFL.  */
+		skip_this_module = true;
+	      }
+	  }
+      if (skip_this_module)
+	return finish ();
+    }
 
   /* Our return value now says to skip the segments contained
      within the module.  */

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]