This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB 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]

[patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" access]


Hi Ulrich,

On Fri, 07 Oct 2011 15:53:09 +0200, Ulrich Weigand wrote:
> On the host system, GDB will take the base filename from the .gnu_debuglink
> section and search a variety of paths for the debuginfo file.

<bite>
This is only because the distro still does not use the normal modern way of
/usr/lib/debug/.build-id/ way to locate the debuginfo file.
</bite>


> For this purpose, GDB will consider the two files identical if either of
> those two checks succeed: the filenames are identical, or "stat" on both
> files returns an identical device/inode pair.  Note that the "stat" check
> was added by Jan to fix this very problem on (native) Debian:
> http://sourceware.org/ml/gdb-patches/2009-11/msg00048.html

<bite>
I see Debian still has not fixed this problem.
</bite>


> Normally, the "stat" check would still catch this situation. However, with
> "remote:" access, "stat" is not implemented.
> 
> To fix this, either of the following two approaches could be employed:
> - Implement "stat" for the "remote:" file access protocol (but this would
>   imply extending the remote protocol, and wouldn't help with old gdbservers
>   on the other side)
> - Omit the potentially spurious warning if the remote protocol is used to
>   access the file (but this would also omit the warning if we get a real
>   debuginfo mismatch due to out-of-date debuginfo)
> 
> Any thoughs?  Am I missing another option here?

Is acceptable the way below?  It is only backward compatibility fallback for
systems still not using both .build-id and the .debug suffix, any of those two
fixes would suffice.

It is backward compatible with existing gdbserver so it can be used as
a fallback.  If the performance hit is bad even for this fallback case all the
other methods failed I would be for gdbserver protocol extension for
these-two-files-are-the-same, used if both files are "remote:".

No regressions on {x86_64,x86_64-m32,i686}-fedora16pre-linux-gnu.


Thanks,
Jan


gdb/
2011-10-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix separate debuginfo warning with "remote:" access.
	* symfile.c (get_file_crc): New function with the code moved from ...
	(separate_debug_file_exists): ... this function, specifically variables
	buffer and count.  New variable verified_as_different, set it.  Remove
	file_crc initialization.  Verify also if both files are not the same
	manually, if needed.

--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1310,15 +1310,29 @@ get_debug_link_info (struct objfile *objfile, unsigned long *crc32_out)
   return contents;
 }
 
+/* Return 32-bit CRC for ABFD.  ABFD must have its file position at start.  */
+
+static unsigned long
+get_file_crc (bfd *abfd)
+{
+  unsigned long file_crc = 0;
+  gdb_byte buffer[8 * 1024];
+  int count;
+
+  while ((count = bfd_bread (buffer, sizeof (buffer), abfd)) > 0)
+    file_crc = gnu_debuglink_crc32 (file_crc, buffer, count);
+
+  return file_crc;
+}
+
 static int
 separate_debug_file_exists (const char *name, unsigned long crc,
 			    struct objfile *parent_objfile)
 {
-  unsigned long file_crc = 0;
+  unsigned long file_crc;
   bfd *abfd;
-  gdb_byte buffer[8*1024];
-  int count;
   struct stat parent_stat, abfd_stat;
+  int verified_as_different;
 
   /* Find a separate debug info file as if symbols would be present in
      PARENT_OBJFILE itself this function would not be called.  .gnu_debuglink
@@ -1345,25 +1359,35 @@ separate_debug_file_exists (const char *name, unsigned long crc,
      negatives.  */
 
   if (bfd_stat (abfd, &abfd_stat) == 0
-      && bfd_stat (parent_objfile->obfd, &parent_stat) == 0
-      && abfd_stat.st_dev == parent_stat.st_dev
-      && abfd_stat.st_ino == parent_stat.st_ino
-      && abfd_stat.st_ino != 0)
+      && abfd_stat.st_ino != 0
+      && bfd_stat (parent_objfile->obfd, &parent_stat) == 0)
     {
-      bfd_close (abfd);
-      return 0;
+      if (abfd_stat.st_dev == parent_stat.st_dev
+	  && abfd_stat.st_ino == parent_stat.st_ino)
+	{
+	  bfd_close (abfd);
+	  return 0;
+	}
+      verified_as_different = 1;
     }
+  else
+    verified_as_different = 0;
 
-  while ((count = bfd_bread (buffer, sizeof (buffer), abfd)) > 0)
-    file_crc = gnu_debuglink_crc32 (file_crc, buffer, count);
+  file_crc = get_file_crc (abfd);
 
   bfd_close (abfd);
 
   if (crc != file_crc)
     {
-      warning (_("the debug information found in \"%s\""
-		 " does not match \"%s\" (CRC mismatch).\n"),
-	       name, parent_objfile->name);
+      /* If one (or both) the files are accessed for example the via "remote:"
+	 gdbserver way it does not support the bfd_stat operation.  Verify
+	 whether those two files are not the same manually.  */
+      if (verified_as_different
+	  || bfd_seek (parent_objfile->obfd, 0, SEEK_SET) != 0
+	  || get_file_crc (parent_objfile->obfd) != crc)
+	warning (_("the debug information found in \"%s\""
+		   " does not match \"%s\" (CRC mismatch).\n"),
+		 name, parent_objfile->name);
       return 0;
     }
 


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