Recent separate debug file warning caused Debian regressions

Recent separate debug file warning caused Debian regressions

[Daniel Jacobowitz]

One of your patches in the last few days caused at least these
regressions for me:

-PASS: gdb.base/annota1.exp: run until main breakpoint
+FAIL: gdb.base/annota1.exp: run until main breakpoint

 Running /space/fsf/commit/src/gdb/testsuite/gdb.mi/mi-async.exp ...
+FAIL: gdb.mi/mi-async.exp: start: send (failed to resume)
 PASS: gdb.mi/mi-async.exp: start: stop
 PASS: gdb.mi/mi-async.exp: CLI next: stop
+FAIL: gdb.mi/mi-async.exp: restart: send (failed to resume)
 PASS: gdb.mi/mi-async.exp: restart: stop

-PASS: gdb.mi/mi-nonstop-exit.exp: mi runto main
-PASS: gdb.mi/mi-nonstop-exit.exp: finished exec continue
-PASS: gdb.mi/mi-nonstop-exit.exp: breakpoint at main
-PASS: gdb.mi/mi-nonstop-exit.exp: mi runto main
-PASS: gdb.mi/mi-nonstop-exit.exp: finished exec continue (2)
+ERROR: Unable to start target
+ERROR: mi-nonstop-exit.exp tests suppressed

It's definitely one of the last four patches you committed, and I
suspect it's the warning patch.  I believe the problem is this
message:

warning: the debug information found in "/lib/libm-2.9.so" does not
match "/lib/libm.so.6" (CRC mismatch).

It's upsetting the testsuite.  I don't know why we're calculating a
CRC for /lib/libm-2.9.so, which is of course the same library - not
a separate debug file - so no wonder the CRC doesn't match, it's the
CRC of the debug info.

1421      /* First try in the same directory as the original file.  */
1422      strcpy (debugfile, dir);
1423      strcat (debugfile, basename);

It comes from this check.  We don't even compare the name (which is
likely to be the same, but in the case of GLIBC's symlinks, isn't).
Should we use stat to reject CRC'ing the file itself?  I know there's
a catch with inode number checks on some filesystems (Windows), but ld
has some code for this.

-- 
Daniel Jacobowitz
CodeSourcery

[patch] Fix false separate debuginfo warning for no .debug suffix [Re: Recent separate debug file warning caused Debian regressions]

[Jan Kratochvil]

On Mon, 02 Nov 2009 22:52:55 +0100, Daniel Jacobowitz wrote:
> It's upsetting the testsuite.  I don't know why we're calculating a
> CRC for /lib/libm-2.9.so, which is of course the same library - not
> a separate debug file - so no wonder the CRC doesn't match, it's the
> CRC of the debug info.

I did not know there exists .gnu_debuglink with different content than the
".debug" suffix:

objdump -s -j .gnu_debuglink libc-debian.so
 0000 6c696263 2d322e31 302e312e 736f0000  libc-2.10.1.so..
 0010 f4f73635                             ..65            

objdump -s -j .gnu_debuglink libc-fedora.so
 0000 6c696263 2d322e31 302e312e 736f2e64  libc-2.10.1.so.d
 0010 65627567 00000000 7245ef0a           ebug....rE..    


> It comes from this check.  We don't even compare the name (which is
> likely to be the same, but in the case of GLIBC's symlinks, isn't).
> Should we use stat to reject CRC'ing the file itself?

Used both for the best performance - the number of syscalls called.


> I know there's a catch with inode number checks on some filesystems
> (Windows), but ld has some code for this.

Thanks, I did not know, included.

Regression tested on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
Verified the behavior on `libc6_2.10.1-5_amd64.deb'.

Patch requires:
	[patch] AC_SYS_LARGEFILE
	http://sourceware.org/ml/gdb-patches/2009-11/msg00047.html


Sorry,
Jan


gdb/
2009-11-03  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symfile.c (separate_debug_file_exists): Change parameter parent_name
	to parent_objfile.  New variables parent_stat and abfd_stat.  Call
	strcmp and then bfd_stat functions to verify if NAME matches.
	(find_separate_debug_file): Update the passed parameter at caller.

--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1308,12 +1308,22 @@ get_debug_link_info (struct objfile *objfile, unsigned long *crc32_out)
 
 static int
 separate_debug_file_exists (const char *name, unsigned long crc,
-			    const char *parent_name)
+			    struct objfile *parent_objfile)
 {
   unsigned long file_crc = 0;
   bfd *abfd;
   gdb_byte buffer[8*1024];
   int count;
+  struct stat parent_stat, abfd_stat;
+
+  /* Find a separate debug info file as if symbols would be present in
+     PARENT_OBJFILE itself this function would not be called.  .gnu_debuglink
+     section can contain just the basename of PARENT_OBJFILE without any
+     ".debug" suffix as "/usr/lib/debug/path/to/file" is a separate tree where
+     the separate debug infos with the same basename can exist. */
+
+  if (strcmp (name, parent_objfile->name) == 0)
+    return 0;
 
   if (remote_filename_p (name))
     abfd = remote_bfd_open (name, gnutarget);
@@ -1323,6 +1333,26 @@ separate_debug_file_exists (const char *name, unsigned long crc,
   if (!abfd)
     return 0;
 
+  /* Verify symlinks were not the cause of strcmp name difference above.
+
+     Some operating systems, e.g. Windows, do not provide a meaningful
+     st_ino; they always set it to zero.  (Windows does provide a
+     meaningful st_dev.)  Do not indicate a duplicate library in that
+     case.  While there is no guarantee that a system that provides
+     meaningful inode numbers will never set st_ino to zero, this is
+     merely an optimization, so we do not need to worry about false
+     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)
+    {
+      bfd_close (abfd);
+      return 0;
+    }
+
   while ((count = bfd_bread (buffer, sizeof (buffer), abfd)) > 0)
     file_crc = gnu_debuglink_crc32 (file_crc, buffer, count);
 
@@ -1332,7 +1353,7 @@ separate_debug_file_exists (const char *name, unsigned long crc,
     {
       warning (_("the debug information found in \"%s\""
 		 " does not match \"%s\" (CRC mismatch).\n"),
-	       name, parent_name);
+	       name, parent_objfile->name);
       return 0;
     }
 
@@ -1422,7 +1443,7 @@ find_separate_debug_file (struct objfile *objfile)
   strcpy (debugfile, dir);
   strcat (debugfile, basename);
 
-  if (separate_debug_file_exists (debugfile, crc32, objfile->name))
+  if (separate_debug_file_exists (debugfile, crc32, objfile))
     goto cleanup_return_debugfile;
 
   /* Then try in the subdirectory named DEBUG_SUBDIRECTORY.  */
@@ -1431,7 +1452,7 @@ find_separate_debug_file (struct objfile *objfile)
   strcat (debugfile, "/");
   strcat (debugfile, basename);
 
-  if (separate_debug_file_exists (debugfile, crc32, objfile->name))
+  if (separate_debug_file_exists (debugfile, crc32, objfile))
     goto cleanup_return_debugfile;
 
   /* Then try in the global debugfile directories.
@@ -1457,7 +1478,7 @@ find_separate_debug_file (struct objfile *objfile)
       strcat (debugfile, dir);
       strcat (debugfile, basename);
 
-      if (separate_debug_file_exists (debugfile, crc32, objfile->name))
+      if (separate_debug_file_exists (debugfile, crc32, objfile))
 	goto cleanup_return_debugfile;
 
       /* If the file is in the sysroot, try using its base path in the
@@ -1472,7 +1493,7 @@ find_separate_debug_file (struct objfile *objfile)
 	  strcat (debugfile, "/");
 	  strcat (debugfile, basename);
 
-	  if (separate_debug_file_exists (debugfile, crc32, objfile->name))
+	  if (separate_debug_file_exists (debugfile, crc32, objfile))
 	    goto cleanup_return_debugfile;
 	}
 

Re: [patch] Fix false separate debuginfo warning for no .debug suffix [Re: Recent separate debug file warning caused Debian regressions]

[Daniel Jacobowitz]

On Tue, Nov 03, 2009 at 11:44:19AM +0100, Jan Kratochvil wrote:
> gdb/
> 2009-11-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* symfile.c (separate_debug_file_exists): Change parameter parent_name
> 	to parent_objfile.  New variables parent_stat and abfd_stat.  Call
> 	strcmp and then bfd_stat functions to verify if NAME matches.
> 	(find_separate_debug_file): Update the passed parameter at caller.

Thanks!  This is OK.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix false separate debuginfo warning for no .debug suffix [Re: Recent separate debug file warning caused Debian regressions]

[Jan Kratochvil]

On Tue, 03 Nov 2009 18:00:04 +0100, Daniel Jacobowitz wrote:
> On Tue, Nov 03, 2009 at 11:44:19AM +0100, Jan Kratochvil wrote:
> > gdb/
> > 2009-11-03  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* symfile.c (separate_debug_file_exists): Change parameter parent_name
> > 	to parent_objfile.  New variables parent_stat and abfd_stat.  Call
> > 	strcmp and then bfd_stat functions to verify if NAME matches.
> > 	(find_separate_debug_file): Update the passed parameter at caller.
> 
> Thanks!  This is OK.

FYI now waiting on getting resolved:
	[patch, configures] Unify AC_SYS_LARGEFILE exception across dirs
	http://gcc.gnu.org/ml/gcc-patches/2009-11/msg00230.html

for a proper form of check-in for:
	[patch] AC_SYS_LARGEFILE
	http://sourceware.org/ml/gdb-patches/2009-11/msg00047.html

which is a prerequisite of this patch.  I hope the regression is not so urgent
to require a temporary workaround instead.


Sorry,
Jan

Re: [patch] Fix false separate debuginfo warning for no .debug suffix [Re: Recent separate debug file warning caused Debian regressions]

[Daniel Jacobowitz]

On Thu, Nov 05, 2009 at 12:26:43AM +0100, Jan Kratochvil wrote:
> for a proper form of check-in for:
> 	[patch] AC_SYS_LARGEFILE
> 	http://sourceware.org/ml/gdb-patches/2009-11/msg00047.html
> 
> which is a prerequisite of this patch.  I hope the regression is not so urgent
> to require a temporary workaround instead.

Thanks for letting me know.  I think it's fine; I can just ignore the
failures for a little longer...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix false separate debuginfo warning for no .debug suffix [Re: Recent separate debug file warning caused Debian regressions]

[Jan Kratochvil]

On Thu, 05 Nov 2009 21:03:55 +0100, Daniel Jacobowitz wrote:
> On Thu, Nov 05, 2009 at 12:26:43AM +0100, Jan Kratochvil wrote:
> > for a proper form of check-in for:
> > 	[patch] AC_SYS_LARGEFILE
> > 	http://sourceware.org/ml/gdb-patches/2009-11/msg00047.html
> > 
> > which is a prerequisite of this patch.  I hope the regression is not so urgent
> > to require a temporary workaround instead.
> 
> Thanks for letting me know.  I think it's fine; I can just ignore the
> failures for a little longer...

Checked-in:
	http://sourceware.org/ml/gdb-cvs/2009-11/msg00081.html

The regression on Debian should be fixed now.


Sorry,
Jan