* [rfc] False separate debuginfo warning with "remote:" access
@ 2011-10-07 13:53 Ulrich Weigand
2011-10-10 8:38 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" access] Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Ulrich Weigand @ 2011-10-07 13:53 UTC (permalink / raw)
To: gdb-patches; +Cc: jan.kratochvil
Hello,
when doing cross-testing using a GDB built with --with-sysroot=remote:
against an Ubuntu ARM distribution, test cases will fail due to spurious
warnings along the lines of:
warning: the debug information found in
"remote:/lib/arm-linux-gnueabi/libm-2.13.so" does not match
"remote:/lib/arm-linux-gnueabi/libm.so.6" (CRC mismatch).
This is due to a combination of factors:
On the target system, /lib/arm-linux-gnueabi/libm.so.6 is a symlink to
libm-2.13.so. That file /lib/arm-linux-gnueabi/libm-2.13.so had its
debuginfo stripped and carries a .gnu_debuglink section pointing to
"libm-2.13.so". The separate debuginfo is in a file (that would be
installed to) /usr/lib/debug/lib/arm-linux-gnueabi/libm-2.13.so
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. These include
/lib/arm-linux-gnueabi itself, as well as /lib/arm-linux-gnueabi/.debug and
/usr/lib/debug/lib/arm-linux-gnueabi.
Due to the combination of the factors that the debuginfo file may have the
same basename as the underlying shared library file, and that the debuginfo
directory search path includes the directory containing the underlying
shared library file, it may happen that one of the files considered as
potential debuginfo file is actually the original shared library itself.
Since GDB will then check CRCs and determine that the shared library is
not actually itself the debuginfo file, we would get a warning as above.
To avoid this spurious warning, GDB will omit printing it if it detects
the situation that the attempted debuginfo is actually the underlying
library.
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
Usually, the filename check will suffice (and this works with "remote:"
access too). However, the filename check fails if the shared library is
referenced via a symbolic link as in this case. 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?
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" access]
2011-10-07 13:53 [rfc] False separate debuginfo warning with "remote:" access Ulrich Weigand
@ 2011-10-10 8:38 ` Jan Kratochvil
2011-10-10 8:49 ` Jan Kratochvil
2011-10-10 13:48 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" Ulrich Weigand
0 siblings, 2 replies; 9+ messages in thread
From: Jan Kratochvil @ 2011-10-10 8:38 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
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;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" access]
2011-10-10 8:38 ` [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-10 8:49 ` Jan Kratochvil
2011-10-10 13:48 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" Ulrich Weigand
1 sibling, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2011-10-10 8:49 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Mon, 10 Oct 2011 10:37:38 +0200, Jan Kratochvil wrote:
> 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>
If the debuginfo file is not installed at all the false warning would be still
present even on .build-id system (which does not use the .debug suffix).
So .build-id does not fix it.
Regards,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:"
2011-10-10 8:38 ` [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-10 8:49 ` Jan Kratochvil
@ 2011-10-10 13:48 ` Ulrich Weigand
2011-10-10 20:22 ` Jan Kratochvil
1 sibling, 1 reply; 9+ messages in thread
From: Ulrich Weigand @ 2011-10-10 13:48 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> On Fri, 07 Oct 2011 15:53:09 +0200, Ulrich Weigand wrote:
> > 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:".
Right, that's a good solution. If there are indeed performance issues,
I think we should then implement "stat" in the remote protocol, because:
- this already fits into the existing BFD callback structure, and
- it would provide correct file lengths, which some debug info readers
(notably stabs) use as sanity checks to ensure references fall into
the file
> 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.
Just a couple comments on the patch itself:
> +/* Return 32-bit CRC for ABFD. ABFD must have its file position at start. */
From an API simplicity point of view, I'd somewhat prefer for the function to
rewind the file position itself instead of requiring the caller to do it ...
> @@ -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;
So this no longer handles the st_ino == 0 case. I think we still need to do
that, to cope with filesystems (e.g. on Windows?) where stat works, but does
not provide inode numbers ... Two files with zero st_ino should not be
considered equal.
> 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);
As another performance optimization, maybe it would make sense to cache
the parent's CRC (e.g. in the objfile) to avoid redundant computation?
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:"
2011-10-10 13:48 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" Ulrich Weigand
@ 2011-10-10 20:22 ` Jan Kratochvil
2011-10-10 21:23 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remot Ulrich Weigand
2011-10-11 19:07 ` [commit] Fix condition (Re: [patch] Verify byte-by-byte if both files are the same on "remote:") Ulrich Weigand
0 siblings, 2 replies; 9+ messages in thread
From: Jan Kratochvil @ 2011-10-10 20:22 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Mon, 10 Oct 2011 15:47:57 +0200, Ulrich Weigand wrote:
> >From an API simplicity point of view, I'd somewhat prefer for the function to
> rewind the file position itself instead of requiring the caller to do it ...
done, yes, I had some wrong performance assumptions.
> > @@ -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;
>
> So this no longer handles the st_ino == 0 case. I think we still need to do
> that, to cope with filesystems (e.g. on Windows?) where stat works, but does
> not provide inode numbers ... Two files with zero st_ino should not be
> considered equal.
I believe the code is right. st_ino == 0 will verified_as_different = 0,
therefore this code does not make any assumption about such files.
FYI I reordered the condition for some negligible btter performance.
> As another performance optimization, maybe it would make sense to cache
> the parent's CRC (e.g. in the objfile) to avoid redundant computation?
OK, done.
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.
* objfiles.h (struct objfile): New fields crc32 and crc32_p.
* 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/objfiles.h
+++ b/gdb/objfiles.h
@@ -244,6 +244,11 @@ struct objfile
long mtime;
+ /* Cached 32-bit CRC as computed by gnu_debuglink_crc32. CRC32 is valid
+ iff CRC32_P. */
+ unsigned long crc32;
+ int crc32_p;
+
/* Obstack to hold objects that should be freed when we load a new symbol
table from this object file. */
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1310,15 +1310,52 @@ get_debug_link_info (struct objfile *objfile, unsigned long *crc32_out)
return contents;
}
+/* Return 32-bit CRC for ABFD. If successful store it to *FILE_CRC_RETURN and
+ return 1. Otherwise print a warning and return 0. ABFD seek position is
+ not preserved. */
+
+static int
+get_file_crc (bfd *abfd, unsigned long *file_crc_return)
+{
+ unsigned long file_crc = 0;
+
+ if (bfd_seek (abfd, 0, SEEK_SET) != 0)
+ {
+ warning (_("Problem reading \"%s\" for CRC: %s"),
+ bfd_get_filename (abfd), bfd_errmsg (bfd_get_error ()));
+ return 0;
+ }
+
+ for (;;)
+ {
+ gdb_byte buffer[8 * 1024];
+ bfd_size_type count;
+
+ count = bfd_bread (buffer, sizeof (buffer), abfd);
+ if (count == (bfd_size_type) -1)
+ {
+ warning (_("Problem reading \"%s\" for CRC: %s"),
+ bfd_get_filename (abfd), bfd_errmsg (bfd_get_error ()));
+ return 0;
+ }
+ if (count == 0)
+ break;
+ file_crc = gnu_debuglink_crc32 (file_crc, buffer, count);
+ }
+
+ *file_crc_return = file_crc;
+ return 1;
+}
+
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;
+ int file_crc_p;
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 +1382,46 @@ 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_p = get_file_crc (abfd, &file_crc);
bfd_close (abfd);
+ if (!file_crc_p)
+ return 0;
+
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 && !parent_objfile->crc32_p)
+ {
+ parent_objfile->crc32_p = get_file_crc (parent_objfile->obfd,
+ &parent_objfile->crc32);
+ if (!parent_objfile->crc32_p)
+ return 0;
+ }
+
+ if (verified_as_different || parent_objfile->crc32 != crc)
+ warning (_("the debug information found in \"%s\""
+ " does not match \"%s\" (CRC mismatch).\n"),
+ name, parent_objfile->name);
+
return 0;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remot
2011-10-10 20:22 ` Jan Kratochvil
@ 2011-10-10 21:23 ` Ulrich Weigand
2011-10-11 12:58 ` Jan Kratochvil
2011-10-11 19:07 ` [commit] Fix condition (Re: [patch] Verify byte-by-byte if both files are the same on "remote:") Ulrich Weigand
1 sibling, 1 reply; 9+ messages in thread
From: Ulrich Weigand @ 2011-10-10 21:23 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> On Mon, 10 Oct 2011 15:47:57 +0200, Ulrich Weigand wrote:
> > So this no longer handles the st_ino == 0 case. I think we still need to do
> > that, to cope with filesystems (e.g. on Windows?) where stat works, but does
> > not provide inode numbers ... Two files with zero st_ino should not be
> > considered equal.
>
> I believe the code is right. st_ino == 0 will verified_as_different = 0,
> therefore this code does not make any assumption about such files.
>
> FYI I reordered the condition for some negligible btter performance.
Yes, you're right. Sorry, I misread the original patch ...
> gdb/
> 2011-10-10 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Fix separate debuginfo warning with "remote:" access.
> * objfiles.h (struct objfile): New fields crc32 and crc32_p.
> * 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.
Looks good to me.
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remot
2011-10-10 21:23 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remot Ulrich Weigand
@ 2011-10-11 12:58 ` Jan Kratochvil
0 siblings, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2011-10-11 12:58 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Mon, 10 Oct 2011 23:23:28 +0200, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> > gdb/
> > 2011-10-10 Jan Kratochvil <jan.kratochvil@redhat.com>
> >
> > Fix separate debuginfo warning with "remote:" access.
> > * objfiles.h (struct objfile): New fields crc32 and crc32_p.
> > * 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.
>
> Looks good to me.
Checked in:
http://sourceware.org/ml/gdb-cvs/2011-10/msg00085.html
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [commit] Fix condition (Re: [patch] Verify byte-by-byte if both files are the same on "remote:")
2011-10-10 20:22 ` Jan Kratochvil
2011-10-10 21:23 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remot Ulrich Weigand
@ 2011-10-11 19:07 ` Ulrich Weigand
2011-10-11 20:06 ` Jan Kratochvil
1 sibling, 1 reply; 9+ messages in thread
From: Ulrich Weigand @ 2011-10-11 19:07 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
Jan Kratochvil wrote:
> + if (verified_as_different || parent_objfile->crc32 != crc)
> + warning (_("the debug information found in \"%s\""
> + " does not match \"%s\" (CRC mismatch).\n"),
> + name, parent_objfile->name);
I just noticed that this still causes spurious warnings since the
condition is incorrect: we want to skip the warning if the parent's
CRC matches the (potential) debuginfo *file* CRC, not the CRC the
debuginfo is supposed to have.
Fixed by the patch below. Tested on arm-linux-gnueabi in remote
testing using --with-sysroot=remote: with no more spurious CRC
mismatch warnings. Committed to mainline as obvious.
Bye,
Ulrich
ChangeLog:
* symfile.c (separate_debug_file_exists): Fix condition.
Index: gdb/symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.318
diff -u -p -r1.318 symfile.c
--- gdb/symfile.c 11 Oct 2011 12:58:08 -0000 1.318
+++ gdb/symfile.c 11 Oct 2011 18:33:12 -0000
@@ -1418,7 +1418,7 @@ separate_debug_file_exists (const char *
return 0;
}
- if (verified_as_different || parent_objfile->crc32 != crc)
+ if (verified_as_different || parent_objfile->crc32 != file_crc)
warning (_("the debug information found in \"%s\""
" does not match \"%s\" (CRC mismatch).\n"),
name, parent_objfile->name);
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [commit] Fix condition (Re: [patch] Verify byte-by-byte if both files are the same on "remote:")
2011-10-11 19:07 ` [commit] Fix condition (Re: [patch] Verify byte-by-byte if both files are the same on "remote:") Ulrich Weigand
@ 2011-10-11 20:06 ` Jan Kratochvil
0 siblings, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2011-10-11 20:06 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On Tue, 11 Oct 2011 21:06:43 +0200, Ulrich Weigand wrote:
> I just noticed that this still causes spurious warnings since the
> condition is incorrect: we want to skip the warning if the parent's
> CRC matches the (potential) debuginfo *file* CRC, not the CRC the
> debuginfo is supposed to have.
I agree, sorry, I admit I did not try to test it, I should have asked for
a test.
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-10-11 20:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-07 13:53 [rfc] False separate debuginfo warning with "remote:" access Ulrich Weigand
2011-10-10 8:38 ` [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-10 8:49 ` Jan Kratochvil
2011-10-10 13:48 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remote:" Ulrich Weigand
2011-10-10 20:22 ` Jan Kratochvil
2011-10-10 21:23 ` [patch] Verify byte-by-byte if both files are the same on "remote:" [Re: [rfc] False separate debuginfo warning with "remot Ulrich Weigand
2011-10-11 12:58 ` Jan Kratochvil
2011-10-11 19:07 ` [commit] Fix condition (Re: [patch] Verify byte-by-byte if both files are the same on "remote:") Ulrich Weigand
2011-10-11 20:06 ` Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox