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] build-id .debug files load (like .gnu_debuglink)


Hi,

Patch improves performance of the separate debug info files verification as it
does not need to read and checksum the whole .debug file, it only reads its id.

This part may be questionable:
-  debugfile = find_separate_debug_file (objfile); 
+  /* If the file has its own symbol tables it has no separate debug info.  */
+  if (objfile->psymtabs == NULL)
+    debugfile = find_separate_debug_file (objfile);

So far .gnu_debuglink existed only in the main file (not the .debug file) and
it existed only if the debug info was stripped to a separate file from it.
debug-id exists always and even both in the main file and the .debug file,
therefore the former code above would deadlock in a loop.  Not sure if
`PSYMTABS == NULL' is the right condition.  According to my experiments on
GNU/Linux it should be correct.

--- background:

There is now a new build-id feature since binutils-2.17.50.0.18 with section
`.note.gnu.build-id' used for finding the matching binaries for a core file.
	http://fedoraproject.org/wiki/RolandMcGrath/BuildID

Patch uses the recently approved BFD `.note.gnu.build-id' parsing:
	http://sourceware.org/ml/binutils/2007-08/msg00296.html


Regards,
Jan
2007-08-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in (symfile.o): Update dependencies.
	* symfile.c (symbol_file_add_with_addrs_or_offsets): Initialize the
	DEBUGFILE variable.  FIND_SEPARATE_DEBUG_FILE called only if !PSYMTABS.
	(struct build_id): New structure.
	(build_id_bfd_shdr_get, build_id_verify, build_id_to_filename): New.
	(find_separate_debug_file): New variable BUILD_ID.
	Call BUILD_ID_BFD_SHDR_GET with BUILD_ID_TO_FILENAME as the first try.

2007-08-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* lib/gdb.exp (build_id_debug_filename_get): New function.
	* gdb.base/sepdebug.exp: Reflect the changes in the heading comment.
	Remove the generate DEBUG file for the future testcase runs.
	New testcase for the NT_GNU_BUILD_ID retrieval.
	Move the final testing step to ...
	(test_different_dir): ... a new function.
	New parameter XFAIL to XFAIL all the tests performed.
	New parameter TEST_DIFFERENT_DIR parametrizing the directory.
	New parameter TYPE to PF_PREFIX all the tests performed.

--- gdb/Makefile.in	23 Aug 2007 20:33:48 -0000	1.929
+++ gdb/Makefile.in	24 Aug 2007 17:48:06 -0000
@@ -2731,7 +2731,7 @@ symfile.o: symfile.c $(defs_h) $(bfdlink
 	$(gdb_stabs_h) $(gdb_obstack_h) $(completer_h) $(bcache_h) \
 	$(hashtab_h) $(readline_h) $(gdb_assert_h) $(block_h) \
 	$(gdb_string_h) $(gdb_stat_h) $(observer_h) $(exec_h) \
-	$(parser_defs_h) $(varobj_h)
+	$(parser_defs_h) $(varobj_h) $(elf_bfd_h)
 symfile-mem.o: symfile-mem.c $(defs_h) $(symtab_h) $(gdbcore_h) \
 	$(objfiles_h) $(exceptions_h) $(gdbcmd_h) $(target_h) $(value_h) \
 	$(symfile_h) $(observer_h) $(auxv_h) $(elf_common_h)
--- gdb/symfile.c	23 Aug 2007 18:08:39 -0000	1.190
+++ gdb/symfile.c	24 Aug 2007 17:48:07 -0000
@@ -51,6 +51,7 @@
 #include "exec.h"
 #include "parser-defs.h"
 #include "varobj.h"
+#include "elf-bfd.h"
 
 #include <sys/types.h>
 #include <fcntl.h>
@@ -1017,7 +1018,7 @@ symbol_file_add_with_addrs_or_offsets (b
 {
   struct objfile *objfile;
   struct partial_symtab *psymtab;
-  char *debugfile;
+  char *debugfile = NULL;
   struct section_addr_info *orig_addrs = NULL;
   struct cleanup *my_cleanups;
   const char *name = bfd_get_filename (abfd);
@@ -1081,7 +1082,9 @@ symbol_file_add_with_addrs_or_offsets (b
 	}
     }
 
-  debugfile = find_separate_debug_file (objfile);
+  /* If the file has its own symbol tables it has no separate debug info.  */
+  if (objfile->psymtabs == NULL)
+    debugfile = find_separate_debug_file (objfile);
   if (debugfile)
     {
       if (addrs != NULL)
@@ -1223,6 +1226,103 @@ symbol_file_clear (int from_tty)
       printf_unfiltered (_("No symbol file now.\n"));
 }
 
+struct build_id
+  {
+    size_t size;
+    gdb_byte data[1];
+  };
+
+/* Locate NT_GNU_BUILD_ID and return its content.
+   Separate debuginfo files have corrupted PHDR but SHDR is correct there.  */
+
+static struct build_id *
+build_id_bfd_shdr_get (bfd *abfd)
+{
+  struct build_id *retval;
+
+  if (!bfd_check_format (abfd, bfd_object)
+      || bfd_get_flavour (abfd) != bfd_target_elf_flavour
+      || elf_tdata (abfd)->build_id == NULL)
+    return NULL;
+
+  retval = xmalloc (sizeof *retval - 1 + elf_tdata (abfd)->build_id_size);
+  retval->size = elf_tdata (abfd)->build_id_size;
+  memcpy (retval->data, elf_tdata (abfd)->build_id, retval->size);
+
+  return retval;
+}
+
+/* Return if FILENAME has NT_GNU_BUILD_ID matching the CHECK value.  */
+
+static int
+build_id_verify (const char *filename, struct build_id *check)
+{
+  bfd *abfd;
+  struct build_id *found = NULL;
+  int retval = 0;
+
+  /* We expect to be silent on the non-existing files.  */
+  abfd = bfd_openr (filename, gnutarget);
+  if (abfd == NULL)
+    return 0;
+
+  found = build_id_bfd_shdr_get (abfd);
+
+  if (found == NULL)
+    warning (_("File \"%s\" has no build-id, file skipped"), filename);
+  else if (found->size != check->size
+           || memcmp (found->data, check->data, found->size) != 0)
+    warning (_("File \"%s\" has a different build-id, file skipped"), filename);
+  else
+    retval = 1;
+
+  if (!bfd_close (abfd))
+    warning (_("cannot close \"%s\": %s"), filename,
+	     bfd_errmsg (bfd_get_error ()));
+  return retval;
+}
+
+static char *
+build_id_to_filename (struct build_id *build_id, int add_debug_suffix)
+{
+  char *link, *s, *retval = NULL;
+  gdb_byte *data = build_id->data;
+  size_t size = build_id->size;
+
+  /* DEBUG_FILE_DIRECTORY/.build-id/ab/cdef */
+  link = xmalloc (strlen (debug_file_directory) + (sizeof "/.build-id/" - 1) + 1
+		  + 2 * size
+		  + (add_debug_suffix ? sizeof ".debug" - 1 : 0)
+		  + 1);
+  s = link + sprintf (link, "%s/.build-id/", debug_file_directory);
+  if (size > 0)
+    {
+      size--;
+      s += sprintf (s, "%02x", (unsigned) *data++);
+    }
+  if (size > 0)
+    *s++ = '/';
+  while (size-- > 0)
+    s += sprintf (s, "%02x", (unsigned) *data++);
+  if (add_debug_suffix)
+    strcpy (s, ".debug");
+  else
+    *s = 0;
+
+  /* lrealpath() is expensive even for the usually non-existent files.  */
+  if (access (link, F_OK) == 0)
+    retval = lrealpath (link);
+  xfree (link);
+
+  if (retval != NULL && !build_id_verify (retval, build_id))
+    {
+      xfree (retval);
+      retval = NULL;
+    }
+
+  return retval;
+}
+
 static char *
 get_debug_link_info (struct objfile *objfile, unsigned long *crc32_out)
 {
@@ -1300,6 +1400,18 @@ find_separate_debug_file (struct objfile
   bfd_size_type debuglink_size;
   unsigned long crc32;
   int i;
+  struct build_id *build_id;
+
+  build_id = build_id_bfd_shdr_get (objfile->obfd);
+  if (build_id != NULL)
+    {
+      char *build_id_name;
+
+      build_id_name = build_id_to_filename (build_id, 1);
+      free (build_id);
+      if (build_id_name != NULL)
+        return build_id_name;
+    }
 
   basename = get_debug_link_info (objfile, &crc32);
 
--- gdb/testsuite/gdb.base/sepdebug.exp	23 Aug 2007 18:14:17 -0000	1.8
+++ gdb/testsuite/gdb.base/sepdebug.exp	24 Aug 2007 17:48:08 -0000
@@ -19,11 +19,14 @@
 
 # Based on break.exp, written by Rob Savoye. (rob@cygnus.com)
 # Modified to test gdb's handling of separate debug info files.
+# Modified to test gdb's handling of a debug-id retrieval.
 
 # This file has two parts. The first is testing that gdb behaves
 # normally after reading in an executable and its corresponding
 # separate debug file. The second moves the .debug file to a different
 # location and tests the "set debug-file-directory" command.
+# The third is for testing build-id retrievel by finding the separate
+# ".debug-id/ab/cdef.debug" file.
 
 
 if $tracelevel then {
@@ -828,93 +831,152 @@ test_next_with_recursion
 
 #********
 
-# now move the .debug file to a different location so that we can test
-# the "set debug-file-directory" command.
-  
-remote_exec build "mv ${objdir}/${subdir}/.debug/${testfile}.debug ${objdir}/${subdir}"
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_test "set debug-file-directory ${objdir}/${subdir}" ".*" "set separate debug location"
-gdb_load ${binfile}
+proc test_different_dir {type test_different_dir xfail} {
+    global srcdir subdir objdir binfile srcfile timeout gdb_prompt
+    global pf_prefix
+    global bp_location6 decimal hex
+
+    set pf_prefix "$type:"
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_test "set debug-file-directory ${test_different_dir}" ".*" "set separate debug location"
+    gdb_load ${binfile}
 
-if [target_info exists gdb_stub] {
-    gdb_step_for_stub;
-}
+    if [target_info exists gdb_stub] {
+	gdb_step_for_stub;
+    }
 
-#
-# test break at function
-#
-gdb_test "break main" \
-    "Breakpoint.*at.* file .*$srcfile, line.*" \
-    "breakpoint function, optimized file"
+    #
+    # test break at function
+    #
+    if {$xfail} {
+	setup_xfail "*-*-*"
+    }
+    gdb_test "break main" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"breakpoint function, optimized file"
 
-#
-# test break at function
-#
-gdb_test "break marker4" \
-    "Breakpoint.*at.* file .*$srcfile, line.*" \
-    "breakpoint small function, optimized file"
+    #
+    # test break at function
+    #
+    if {$xfail} {
+	setup_xfail "*-*-*"
+    }
+    gdb_test "break marker4" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"breakpoint small function, optimized file"
 
-#
-# run until the breakpoint at main is hit. For non-stubs-using targets.
-#
-gdb_run_cmd
-gdb_expect {
-    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*$gdb_prompt $" {
-	pass "run until function breakpoint, optimized file"
+    #
+    # run until the breakpoint at main is hit. For non-stubs-using targets.
+    #
+    gdb_run_cmd
+    if {$xfail} {
+	setup_xfail "*-*-*"
     }
-    -re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$gdb_prompt $" {
-	pass "run until function breakpoint, optimized file (code motion)"
+    gdb_expect {
+	-re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$srcfile:$bp_location6.*$bp_location6\[\t \]+if .argc.* \{.*$gdb_prompt $" {
+	    pass "run until function breakpoint, optimized file"
+	}
+	-re "Breakpoint \[0-9\]+,.*main .*argc.*argv.* at .*$gdb_prompt $" {
+	    pass "run until function breakpoint, optimized file (code motion)"
+	}
+	-re "$gdb_prompt $" {
+	    fail "run until function breakpoint, optimized file"
+	}
+	timeout {
+	    fail "run until function breakpoint, optimized file (timeout)"
+	}
     }
-    -re "$gdb_prompt $" {
-	fail "run until function breakpoint, optimized file"
+
+    #
+    # run until the breakpoint at a small function
+    #
+
+    #
+    # Add a second pass pattern.  The behavior differs here between stabs
+    # and dwarf for one-line functions.  Stabs preserves two line symbols
+    # (one before the prologue and one after) with the same line number, 
+    # but dwarf regards these as duplicates and discards one of them.
+    # Therefore the address after the prologue (where the breakpoint is)
+    # has no exactly matching line symbol, and GDB reports the breakpoint
+    # as if it were in the middle of a line rather than at the beginning.
+
+    set bp_location13 [gdb_get_line_number "set breakpoint 13 here"]
+    set bp_location14 [gdb_get_line_number "set breakpoint 14 here"]
+    send_gdb "continue\n"
+    if {$xfail} {
+	setup_xfail "*-*-*"
     }
-    timeout {
-	fail "run until function breakpoint, optimized file (timeout)"
+    gdb_expect {
+	-re "Breakpoint $decimal, marker4 \\(d=177601976\\) at .*$srcfile:$bp_location13\[\r\n\]+$bp_location13\[\t \]+void marker4.*" {
+	    pass "run until breakpoint set at small function, optimized file"
+	}
+	-re "Breakpoint $decimal, $hex in marker4 \\(d=177601976\\) at .*$srcfile:$bp_location13\[\r\n\]+$bp_location13\[\t \]+void marker4.*" {
+	    pass "run until breakpoint set at small function, optimized file"
+	}
+	-re "Breakpoint $decimal, marker4 \\(d=177601976\\) at .*$srcfile:$bp_location14\[\r\n\]+$bp_location14\[\t \]+void marker4.*" {
+	    # marker4() is defined at line 46 when compiled with -DPROTOTYPES
+	    pass "run until breakpoint set at small function, optimized file (line bp_location14)"
+	}
+	-re ".*$gdb_prompt " {
+	    fail "run until breakpoint set at small function, optimized file"
+	}
+	timeout {
+	    fail "run until breakpoint set at small function, optimized file (timeout)"
+	}
+    }
+
+
+    # Reset the default arguments for VxWorks
+    if [istarget "*-*-vxworks*"] {
+	set timeout 10
+	verbose "Timeout is now $timeout seconds" 2
+	send_gdb "set args main\n"
+	gdb_expect -re ".*$gdb_prompt $" {}
     }
+
+    unset pf_prefix
+# proc test_different_dir
 }
 
-#
-# run until the breakpoint at a small function
-#
 
-#
-# Add a second pass pattern.  The behavior differs here between stabs
-# and dwarf for one-line functions.  Stabs preserves two line symbols
-# (one before the prologue and one after) with the same line number, 
-# but dwarf regards these as duplicates and discards one of them.
-# Therefore the address after the prologue (where the breakpoint is)
-# has no exactly matching line symbol, and GDB reports the breakpoint
-# as if it were in the middle of a line rather than at the beginning.
+# now move the .debug file to a different location so that we can test
+# the "set debug-file-directory" command.
+  
+remote_exec build "mv ${objdir}/${subdir}/.debug/${testfile}.debug ${objdir}/${subdir}"
+set debugfile "${objdir}/${subdir}/${testfile}.debug"
 
-set bp_location13 [gdb_get_line_number "set breakpoint 13 here"]
-set bp_location14 [gdb_get_line_number "set breakpoint 14 here"]
-send_gdb "continue\n"
-gdb_expect {
-    -re "Breakpoint $decimal, marker4 \\(d=177601976\\) at .*$srcfile:$bp_location13\[\r\n\]+$bp_location13\[\t \]+void marker4.*" {
-	pass "run until breakpoint set at small function, optimized file"
-    }
-    -re "Breakpoint $decimal, $hex in marker4 \\(d=177601976\\) at .*$srcfile:$bp_location13\[\r\n\]+$bp_location13\[\t \]+void marker4.*" {
-	pass "run until breakpoint set at small function, optimized file"
-    }
-    -re "Breakpoint $decimal, marker4 \\(d=177601976\\) at .*$srcfile:$bp_location14\[\r\n\]+$bp_location14\[\t \]+void marker4.*" {
-        # marker4() is defined at line 46 when compiled with -DPROTOTYPES
-	pass "run until breakpoint set at small function, optimized file (line bp_location14)"
-    }
-    -re ".*$gdb_prompt " {
-	fail "run until breakpoint set at small function, optimized file"
-    }
-    timeout {
-	fail "run until breakpoint set at small function, optimized file (timeout)"
+test_different_dir debuglink "${objdir}/${subdir}" 0
+
+
+# NT_GNU_BUILD_ID / .note.gnu.build-id test:
+
+set build_id_debug_filename [build_id_debug_filename_get $binfile]
+if {$build_id_debug_filename eq ""} {
+    unsupported "build-id is not supported by the compiler"
+
+    # Spare debug files may confuse testsuite runs in the future.
+    remote_exec build "rm -f $debugfile"
+} else {
+    set build_id_debugself_filename [build_id_debug_filename_get $debugfile]
+    set test "build-id support by binutils"
+    set xfail 0
+    if {$build_id_debugself_filename eq ""} {
+	unsupported $test
+	set xfail 1
+    } elseif {$build_id_debugself_filename ne $build_id_debug_filename} {
+	fail $test
+    } else {
+	pass $test
     }
-}
 
+    file mkdir [file dirname ${objdir}/${subdir}/${build_id_debug_filename}]
+    remote_exec build "mv $debugfile ${objdir}/${subdir}/${build_id_debug_filename}"
+
+    test_different_dir build-id "${objdir}/${subdir}" $xfail
 
-# Reset the default arguments for VxWorks
-if [istarget "*-*-vxworks*"] {
-    set timeout 10
-    verbose "Timeout is now $timeout seconds" 2
-    send_gdb "set args main\n"
-    gdb_expect -re ".*$gdb_prompt $" {}
+    # Spare debug files may confuse testsuite runs in the future.
+    remote_exec build "rm -f ${objdir}/${subdir}/${build_id_debug_filename}"
 }
--- gdb/testsuite/lib/gdb.exp	23 Aug 2007 20:10:04 -0000	1.86
+++ gdb/testsuite/lib/gdb.exp	24 Aug 2007 17:48:08 -0000
@@ -2482,6 +2482,27 @@ proc separate_debug_filename { exec } {
     return $debug_file
 }
 
+# Return the build-id hex string (usually 160 bits as 40 hex characters)
+# converted to the form: .build-id/ab/cdef1234...89.debug
+# Return "" if no build-id found.
+proc build_id_debug_filename_get { exec } {
+    set tmp "${exec}-tmp"
+    exec objcopy -j .note.gnu.build-id -O binary $exec $tmp
+    set fi [open $tmp]
+    # Skip the NOTE header.
+    read $fi 16
+    set data [read $fi]
+    close $fi
+    file delete $tmp
+    if {$data eq ""} {
+	return ""
+    }
+    # Convert it to hex.
+    binary scan $data H* data
+    set data [regsub {^..} $data {\0/}]
+    return ".build-id/${data}.debug";
+}
+
 # Create stripped files for DEST, replacing it.  If ARGS is passed, it is a
 # list of optional flags.  The only currently supported flag is no-main,
 # which removes the symbol entry for main from the separate debug file.

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