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

Jan Kratochvil
2011-10-10T08:38:32Z

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;
}