public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] fix PR symtab/15597
@ 2013-08-08 17:37 Tom Tromey
  2013-08-10  5:36 ` Alan Modra
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Tom Tromey @ 2013-08-08 17:37 UTC (permalink / raw)
  To: gdb-patches; +Cc: binutils, Tom Tromey

This patch fixes gdb PR symtab/15597.

The bug is that the .gnu_debugaltlink section includes the build-id of
the alt file, but gdb does not use it.

This patch fixes the problem by changing gdb to do what it ought to
always have done: verify the build id of the file found using the
filename in .gnu_debugaltlink; and if that does not match, try to find
the correct debug file using the build-id and debug-file-directory.

This patch touches BFD.  Previously, gdb had its own code for parsing
.gnu_debugaltlink; I changed it to use the BFD functions after those
were introduced.  However, the BFD functions are incorrect -- they
assume that .gnu_debugaltlink is formatted like .gnu_debuglink.
However, it it is not.  Instead, it consists of a file name followed
by the build-id -- no alignment, and the build-id is not a CRC.

Fixing this properly is a bit of a pain.  But, because
separate_alt_debug_file_exists just has a FIXME for the build-id case,
I did not fix it properly.  Instead I introduced a hack.  This leaves
BFD working just as well as it did before my patch.

I'm willing to do something better here but I could use some guidance
as to what.  It seems that the build-id code in BFD is largely punted
on.

FWIW gdb is the only user of bfd_get_alt_debug_link_info outside of
BFD itself.

I moved the build-id logic out of elfread.c and into a new file.
This seemed cleanest to me.

Writing a test case was a bit of a pain.  I added a couple new
features to the DWARF assembler to handle this.

Built and regtested on x86-64 Fedora 18.

	* bfd-in2.h: Rebuild.
	* opncls.c (bfd_get_alt_debug_link_info): Add buildid_len
	parameter.  Change type of buildid_out.  Update.
	(get_alt_debug_link_info_shim): New function.
	(bfd_follow_gnu_debuglink): Use it.

	* Makefile.in (SFILES): Add build-id.c.
	(HFILES_NO_SRCDIR): Add build-id.h.
	* build-id.c: New file, largely from elfread.c.  Modified
	most functions.
	* build-id.h: New file.
	* dwarf2read.c (dwarf2_get_dwz_file): Update for change to
	bfd_get_alt_debug_link_info.  Verify dwz file's build-id.
	Search for dwz file using build-id.
	* elfread.c (build_id_bfd_get, build_id_verify)
	(build_id_to_debug_filename, find_separate_debug_file): Remove.

	* gdb.dwarf2/dwzbuildid.exp: New file.
	* lib/dwarf.exp (Dwarf::_section): Add "flags" and "type"
	parameters.
	(Dwarf::_defer_output): Change "section" parameter to
	"section_spec"; update.
	(Dwarf::gnu_debugaltlink, Dwarf::_note, Dwarf::build_id): New
	procs.
---
 bfd/bfd-in2.h                           |   3 +-
 bfd/opncls.c                            |  41 ++++++--
 gdb/Makefile.in                         |   8 +-
 gdb/build-id.c                          | 167 +++++++++++++++++++++++++++++++
 gdb/build-id.h                          |  44 +++++++++
 gdb/dwarf2read.c                        |  32 +++---
 gdb/elfread.c                           | 126 +----------------------
 gdb/testsuite/gdb.dwarf2/dwzbuildid.exp | 170 ++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp             |  62 +++++++++++-
 9 files changed, 498 insertions(+), 155 deletions(-)
 create mode 100644 gdb/build-id.c
 create mode 100644 gdb/build-id.h
 create mode 100644 gdb/testsuite/gdb.dwarf2/dwzbuildid.exp

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 0cf9a29..986ad0a 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1067,7 +1067,8 @@ unsigned long bfd_calc_gnu_debuglink_crc32
 
 char *bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out);
 
-char *bfd_get_alt_debug_link_info (bfd *abfd, unsigned long *crc32_out);
+char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+    bfd_byte **buildid_out);
 
 char *bfd_follow_gnu_debuglink (bfd *abfd, const char *dir);
 
diff --git a/bfd/opncls.c b/bfd/opncls.c
index cd9c826..f29b2c8 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -1194,18 +1194,21 @@ FUNCTION
 	bfd_get_alt_debug_link_info
 
 SYNOPSIS
-	char *bfd_get_alt_debug_link_info (bfd *abfd, unsigned long *crc32_out);
+	char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+			                   bfd_byte **buildid_out);
 
 DESCRIPTION
 	Fetch the filename and BuildID value for any alternate debuginfo
 	associated with @var{abfd}.  Return NULL if no such info found,
-	otherwise return filename and update @var{buildid_out}.  The
-	returned filename is allocated with @code{malloc}; freeing it
-	is the responsibility of the caller.
+	otherwise return filename and update @var{buildid_len} and
+	@var{buildid_out}.  The returned filename and build_id are
+	allocated with @code{malloc}; freeing them is the
+	responsibility of the caller.
 */
 
 char *
-bfd_get_alt_debug_link_info (bfd * abfd, unsigned long * buildid_out)
+bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+			     bfd_byte **buildid_out)
 {
   asection *sect;
   bfd_byte *contents;
@@ -1213,6 +1216,7 @@ bfd_get_alt_debug_link_info (bfd * abfd, unsigned long * buildid_out)
   char *name;
 
   BFD_ASSERT (abfd);
+  BFD_ASSERT (buildid_len);
   BFD_ASSERT (buildid_out);
 
   sect = bfd_get_section_by_name (abfd, GNU_DEBUGALTLINK);
@@ -1227,12 +1231,13 @@ bfd_get_alt_debug_link_info (bfd * abfd, unsigned long * buildid_out)
       return NULL;
     }
 
-  /* BuildID value is stored after the filename, aligned up to 4 bytes.  */
+  /* BuildID value is stored after the filename.  */
   name = (char *) contents;
   buildid_offset = strlen (name) + 1;
-  buildid_offset = (buildid_offset + 3) & ~3;
 
-  * buildid_out = bfd_get_32 (abfd, contents + buildid_offset);
+  *buildid_len = bfd_get_section_size (sect) - buildid_offset;
+  *buildid_out = bfd_malloc (*buildid_len);
+  memcpy (*buildid_out, contents + buildid_offset, *buildid_len);
 
   return name;
 }
@@ -1466,6 +1471,24 @@ bfd_follow_gnu_debuglink (bfd *abfd, const char *dir)
 				   separate_debug_file_exists);
 }
 
+/* Helper for bfd_follow_gnu_debugaltlink.  It just pretends to return
+   a CRC.  .gnu_debugaltlink supplies a build-id, which is different,
+   but this is ok because separate_alt_debug_file_exists ignores the
+   CRC anyway.  */
+
+static char *
+get_alt_debug_link_info_shim (bfd * abfd, unsigned long *crc32_out)
+{
+  size_t len;
+  bfd_byte *buildid = NULL;
+  char *result = bfd_get_alt_debug_link_info (abfd, &len, &buildid);
+
+  *crc32_out = 0;
+  free (buildid);
+
+  return result;
+}
+
 /*
 FUNCTION
 	bfd_follow_gnu_debugaltlink
@@ -1496,7 +1519,7 @@ char *
 bfd_follow_gnu_debugaltlink (bfd *abfd, const char *dir)
 {
   return find_separate_debug_file (abfd, dir,
-				   bfd_get_alt_debug_link_info,
+				   get_alt_debug_link_info_shim,
 				   separate_alt_debug_file_exists);
 }
 
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8f4ee9e..18d80be 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -717,7 +717,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	bfd-target.c \
 	block.c blockframe.c \
 	breakpoint.c break-catch-sig.c break-catch-throw.c \
-	buildsym.c \
+	build-id.c buildsym.c \
 	c-exp.y c-lang.c c-typeprint.c c-valprint.c \
 	charset.c cleanups.c cli-out.c coffread.c coff-pe-read.c \
 	complaints.c completer.c continuations.c corefile.c corelow.c \
@@ -814,7 +814,8 @@ tui/tui-file.h tui/tui-command.h tui/tui-disasm.h tui/tui-wingeneral.h \
 tui/tui-windata.h tui/tui-data.h tui/tui-win.h tui/tui-stack.h \
 tui/tui-winsource.h tui/tui-regs.h tui/tui-io.h tui/tui-layout.h \
 tui/tui-source.h sol2-tdep.h gregset.h sh-tdep.h sh64-tdep.h \
-expression.h score-tdep.h gdb_select.h ser-tcp.h buildsym.h valprint.h \
+expression.h score-tdep.h gdb_select.h ser-tcp.h \
+build-id.h buildsym.h valprint.h \
 typeprint.h mi/mi-getopt.h mi/mi-parse.h mi/mi-console.h \
 mi/mi-out.h mi/mi-main.h mi/mi-common.h mi/mi-cmds.h linux-nat.h \
 complaints.h gdb_proc_service.h gdb_regex.h xtensa-tdep.h inf-loop.h \
@@ -909,7 +910,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	event-loop.o event-top.o inf-loop.o completer.o \
 	gdbarch.o arch-utils.o gdbtypes.o gdb_bfd.o gdb_obstack.o \
 	osabi.o copying.o \
-	memattr.o mem-break.o target.o parse.o language.o buildsym.o \
+	memattr.o mem-break.o target.o parse.o language.o \
+	build-id.o buildsym.o \
 	findcmd.o \
 	std-regs.o \
 	signals.o \
diff --git a/gdb/build-id.c b/gdb/build-id.c
new file mode 100644
index 0000000..d26a791
--- /dev/null
+++ b/gdb/build-id.c
@@ -0,0 +1,167 @@
+/* build-id-related functions.
+
+   Copyright (C) 1991-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "bfd.h"
+#include "elf-bfd.h"
+#include "gdb_bfd.h"
+#include "build-id.h"
+#include "gdb_string.h"
+#include "gdb_vecs.h"
+#include "symfile.h"
+#include "objfiles.h"
+#include "filenames.h"
+
+/* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
+
+static const struct elf_build_id *
+build_id_bfd_get (bfd *abfd)
+{
+  if (!bfd_check_format (abfd, bfd_object)
+      || bfd_get_flavour (abfd) != bfd_target_elf_flavour
+      /* Although this is ELF_specific, it is safe to do in generic
+	 code because it does not rely on any ELF-specific symbols at
+	 link time, and if the ELF code is not available in BFD, then
+	 ABFD will not have the ELF flavour.  */
+      || elf_tdata (abfd)->build_id == NULL)
+    return NULL;
+
+  return elf_tdata (abfd)->build_id;
+}
+
+/* See build-id.h.  */
+
+int
+build_id_verify (bfd *abfd, size_t check_len, const bfd_byte *check)
+{
+  const struct elf_build_id *found;
+  int retval = 0;
+
+  found = build_id_bfd_get (abfd);
+
+  if (found == NULL)
+    warning (_("File \"%s\" has no build-id, file skipped"),
+	     bfd_get_filename (abfd));
+  else if (found->size != check_len
+           || memcmp (found->data, check, found->size) != 0)
+    warning (_("File \"%s\" has a different build-id, file skipped"),
+	     bfd_get_filename (abfd));
+  else
+    retval = 1;
+
+  return retval;
+}
+
+/* See build-id.h.  */
+
+bfd *
+build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id)
+{
+  char *link, *debugdir;
+  VEC (char_ptr) *debugdir_vec;
+  struct cleanup *back_to;
+  int ix;
+  bfd *abfd = NULL;
+
+  /* DEBUG_FILE_DIRECTORY/.build-id/ab/cdef */
+  link = alloca (strlen (debug_file_directory) + (sizeof "/.build-id/" - 1) + 1
+		 + 2 * build_id_len + (sizeof ".debug" - 1) + 1);
+
+  /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
+     cause "/.build-id/..." lookups.  */
+
+  debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
+  back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
+
+  for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
+    {
+      size_t debugdir_len = strlen (debugdir);
+      const gdb_byte *data = build_id;
+      size_t size = build_id_len;
+      char *s;
+      char *filename = NULL;
+
+      memcpy (link, debugdir, debugdir_len);
+      s = &link[debugdir_len];
+      s += sprintf (s, "/.build-id/");
+      if (size > 0)
+	{
+	  size--;
+	  s += sprintf (s, "%02x", (unsigned) *data++);
+	}
+      if (size > 0)
+	*s++ = '/';
+      while (size-- > 0)
+	s += sprintf (s, "%02x", (unsigned) *data++);
+      strcpy (s, ".debug");
+
+      /* lrealpath() is expensive even for the usually non-existent files.  */
+      if (access (link, F_OK) == 0)
+	filename = lrealpath (link);
+
+      if (filename == NULL)
+	continue;
+
+      /* We expect to be silent on the non-existing files.  */
+      abfd = gdb_bfd_open_maybe_remote (filename);
+      if (abfd == NULL)
+	continue;
+
+      if (build_id_verify (abfd, build_id_len, build_id))
+	break;
+
+      gdb_bfd_unref (abfd);
+      abfd = NULL;
+    }
+
+  do_cleanups (back_to);
+  return abfd;
+}
+
+/* See build-id.h.  */
+
+char *
+find_separate_debug_file_by_buildid (struct objfile *objfile)
+{
+  const struct elf_build_id *build_id;
+
+  build_id = build_id_bfd_get (objfile->obfd);
+  if (build_id != NULL)
+    {
+      bfd *abfd;
+
+      abfd = build_id_to_debug_bfd (build_id->size, build_id->data);
+      /* Prevent looping on a stripped .debug file.  */
+      if (abfd != NULL
+	  && filename_cmp (bfd_get_filename (abfd), objfile->name) == 0)
+        {
+	  warning (_("\"%s\": separate debug info file has no debug info"),
+		   bfd_get_filename (abfd));
+	  gdb_bfd_unref (abfd);
+	}
+      else if (abfd != NULL)
+	{
+	  char *result = xstrdup (bfd_get_filename (abfd));
+
+	  gdb_bfd_unref (abfd);
+	  return result;
+	}
+    }
+  return NULL;
+}
diff --git a/gdb/build-id.h b/gdb/build-id.h
new file mode 100644
index 0000000..b3ea4fb
--- /dev/null
+++ b/gdb/build-id.h
@@ -0,0 +1,44 @@
+/* build-id-related functions.
+
+   Copyright (C) 1991-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef BUILD_ID_H
+#define BUILD_ID_H
+
+/* Return true if ABFD has NT_GNU_BUILD_ID matching the CHECK value.
+   Otherwise, issue a warning and return false.  */
+
+extern int build_id_verify (bfd *abfd,
+			    size_t check_len, const bfd_byte *check);
+
+
+/* Find and open a BFD given a build-id.  If no BFD can be found,
+   return NULL.  The returned reference to the BFD must be released by
+   the caller.  */
+
+extern bfd *build_id_to_debug_bfd (size_t build_id_len,
+				   const bfd_byte *build_id);
+
+/* Find the separate debug file for OBJFILE, by using the build-id
+   associated with OBJFILE's BFD.  If successful, returns a malloc'd
+   file name for the separate debug file.  The caller must free this.
+   Otherwise, returns NULL.  */
+
+extern char *find_separate_debug_file_by_buildid (struct objfile *objfile);
+
+#endif /* BUILD_ID_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 9e19e78..b31b05e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -69,6 +69,7 @@
 #include "f-lang.h"
 #include "source.h"
 #include "filestuff.h"
+#include "build-id.h"
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -2121,14 +2122,15 @@ dwarf2_get_dwz_file (void)
   struct cleanup *cleanup;
   const char *filename;
   struct dwz_file *result;
-  unsigned long buildid;
+  size_t buildid_len;
+  bfd_byte *buildid;
 
   if (dwarf2_per_objfile->dwz_file != NULL)
     return dwarf2_per_objfile->dwz_file;
 
   bfd_set_error (bfd_error_no_error);
   data = bfd_get_alt_debug_link_info (dwarf2_per_objfile->objfile->obfd,
-				      &buildid);
+				      &buildid_len, &buildid);
   if (data == NULL)
     {
       if (bfd_get_error () == bfd_error_no_error)
@@ -2137,6 +2139,7 @@ dwarf2_get_dwz_file (void)
 	     bfd_errmsg (bfd_get_error ()));
     }
   cleanup = make_cleanup (xfree, data);
+  make_cleanup (xfree, buildid);
 
   filename = (const char *) data;
   if (!IS_ABSOLUTE_PATH (filename))
@@ -2153,20 +2156,25 @@ dwarf2_get_dwz_file (void)
       filename = rel;
     }
 
-  /* The format is just a NUL-terminated file name, followed by the
-     build-id.  For now, though, we ignore the build-id.  */
+  /* First try the file name given in the section.  If that doesn't
+     work, try to use the build-id instead.  */
   dwz_bfd = gdb_bfd_open (filename, gnutarget, -1);
-  if (dwz_bfd == NULL)
-    error (_("could not read '%s': %s"), filename,
-	   bfd_errmsg (bfd_get_error ()));
-
-  if (!bfd_check_format (dwz_bfd, bfd_object))
+  if (dwz_bfd != NULL)
     {
-      gdb_bfd_unref (dwz_bfd);
-      error (_("file '%s' was not usable: %s"), filename,
-	     bfd_errmsg (bfd_get_error ()));
+      if (!build_id_verify (dwz_bfd, buildid_len, buildid))
+	{
+	  gdb_bfd_unref (dwz_bfd);
+	  dwz_bfd = NULL;
+	}
     }
 
+  if (dwz_bfd == NULL)
+    dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
+
+  if (dwz_bfd == NULL)
+    error (_("could not find '.gnu_debugaltlink' file for %s"),
+	   dwarf2_per_objfile->objfile->name);
+
   result = OBSTACK_ZALLOC (&dwarf2_per_objfile->objfile->objfile_obstack,
 			   struct dwz_file);
   result->dwz_bfd = dwz_bfd;
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 1aa10d1..7ea2760 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -45,6 +45,7 @@
 #include "regcache.h"
 #include "bcache.h"
 #include "gdb_bfd.h"
+#include "build-id.h"
 
 extern void _initialize_elfread (void);
 
@@ -1087,131 +1088,6 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
   update_breakpoint_locations (b, sals, sals_end);
 }
 
-/* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
-
-static const struct elf_build_id *
-build_id_bfd_get (bfd *abfd)
-{
-  if (!bfd_check_format (abfd, bfd_object)
-      || bfd_get_flavour (abfd) != bfd_target_elf_flavour
-      || elf_tdata (abfd)->build_id == NULL)
-    return NULL;
-
-  return elf_tdata (abfd)->build_id;
-}
-
-/* Return if FILENAME has NT_GNU_BUILD_ID matching the CHECK value.  */
-
-static int
-build_id_verify (const char *filename, const struct elf_build_id *check)
-{
-  bfd *abfd;
-  const struct elf_build_id *found;
-  int retval = 0;
-
-  /* We expect to be silent on the non-existing files.  */
-  abfd = gdb_bfd_open_maybe_remote (filename);
-  if (abfd == NULL)
-    return 0;
-
-  found = build_id_bfd_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;
-
-  gdb_bfd_unref (abfd);
-
-  return retval;
-}
-
-static char *
-build_id_to_debug_filename (const struct elf_build_id *build_id)
-{
-  char *link, *debugdir, *retval = NULL;
-  VEC (char_ptr) *debugdir_vec;
-  struct cleanup *back_to;
-  int ix;
-
-  /* DEBUG_FILE_DIRECTORY/.build-id/ab/cdef */
-  link = alloca (strlen (debug_file_directory) + (sizeof "/.build-id/" - 1) + 1
-		 + 2 * build_id->size + (sizeof ".debug" - 1) + 1);
-
-  /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
-     cause "/.build-id/..." lookups.  */
-
-  debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
-  back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
-
-  for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
-    {
-      size_t debugdir_len = strlen (debugdir);
-      const gdb_byte *data = build_id->data;
-      size_t size = build_id->size;
-      char *s;
-
-      memcpy (link, debugdir, debugdir_len);
-      s = &link[debugdir_len];
-      s += sprintf (s, "/.build-id/");
-      if (size > 0)
-	{
-	  size--;
-	  s += sprintf (s, "%02x", (unsigned) *data++);
-	}
-      if (size > 0)
-	*s++ = '/';
-      while (size-- > 0)
-	s += sprintf (s, "%02x", (unsigned) *data++);
-      strcpy (s, ".debug");
-
-      /* lrealpath() is expensive even for the usually non-existent files.  */
-      if (access (link, F_OK) == 0)
-	retval = lrealpath (link);
-
-      if (retval != NULL && !build_id_verify (retval, build_id))
-	{
-	  xfree (retval);
-	  retval = NULL;
-	}
-
-      if (retval != NULL)
-	break;
-    }
-
-  do_cleanups (back_to);
-  return retval;
-}
-
-static char *
-find_separate_debug_file_by_buildid (struct objfile *objfile)
-{
-  const struct elf_build_id *build_id;
-
-  build_id = build_id_bfd_get (objfile->obfd);
-  if (build_id != NULL)
-    {
-      char *build_id_name;
-
-      build_id_name = build_id_to_debug_filename (build_id);
-      /* Prevent looping on a stripped .debug file.  */
-      if (build_id_name != NULL
-	  && filename_cmp (build_id_name, objfile->name) == 0)
-        {
-	  warning (_("\"%s\": separate debug info file has no debug info"),
-		   build_id_name);
-	  xfree (build_id_name);
-	}
-      else if (build_id_name != NULL)
-        return build_id_name;
-    }
-  return NULL;
-}
-
 /* Scan and build partial symbols for a symbol file.
    We have been initialized by a call to elf_symfile_init, which
    currently does nothing.
diff --git a/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp b/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
new file mode 100644
index 0000000..e6ae881
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
@@ -0,0 +1,170 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+# No remote host testing either.
+if {[is_remote host]} {
+    return 0
+}
+
+
+# Lots of source files since we test a few cases and make new files
+# for each.
+# The tests are:
+#     ok - the main file refers to a dwz and the buildids match
+#     mismatch - the buildids do not match
+#     fallback - the buildids do not match but a match is found via buildid
+standard_testfile main.c \
+    dwzbuildid-ok-base.S dwzbuildid-ok-sep.S \
+    dwzbuildid-mismatch-base.S dwzbuildid-mismatch-sep.S \
+    dwzbuildid-fallback-base.S dwzbuildid-fallback-sep.S \
+    dwzbuildid-fallback-ok.S
+    
+# Write some assembly that just has a .gnu_debugaltlink section.
+proc write_just_debugaltlink {filename dwzname buildid} {
+    set asm_file [standard_output_file $filename]
+
+    Dwarf::assemble $asm_file {
+	upvar dwzname dwzname
+	upvar buildid buildid
+
+	gnu_debugaltlink $dwzname $buildid
+
+	# Only the DWARF reader checks .gnu_debugaltlink, so make sure
+	# there is a bit of DWARF in here.
+	cu {} {
+	    compile_unit {{language @DW_LANG_C}} {
+	    }
+	}
+    }
+}
+
+# Write some DWARF that also sets the buildid.
+proc write_dwarf_file {filename buildid {value 99}} {
+    set asm_file [standard_output_file $filename]
+
+    Dwarf::assemble $asm_file {
+	declare_labels partial_label double_label int_label int_label2
+
+	upvar buildid buildid
+	upvar value value
+
+	build_id $buildid
+
+	cu {} {
+	    compile_unit {{language @DW_LANG_C}} {
+		int_label2: base_type {
+		    {name int}
+		    {byte_size 4 sdata}
+		    {encoding @DW_ATE_signed}
+		}
+
+		constant {
+		    {name the_int}
+		    {type :$int_label2}
+		    {const_value $value data1}
+		}
+	    }
+	}
+    }
+}
+
+if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
+	   object {nodebug}] != "" } {
+    return -1
+}
+
+# The values don't really matter, just whether they are equal.
+set ok_prefix 01
+set ok_suffix 0203040506
+set ok_suffix2 02030405ff
+set ok_buildid ${ok_prefix}${ok_suffix}
+set ok_buildid2 ${ok_prefix}${ok_suffix2}
+set bad_buildid ffffffffffff
+
+set outdir [standard_output_file {}]
+set basedir $outdir/.build-id
+file mkdir $basedir $basedir/$ok_prefix
+
+# Test where the separate debuginfo's buildid matches.
+write_just_debugaltlink $srcfile2 ${binfile}3.o $ok_buildid
+write_dwarf_file $srcfile3 $ok_buildid
+
+# Test where the separate debuginfo's buildid does not match.
+write_just_debugaltlink $srcfile4 ${binfile}5.o $ok_buildid
+write_dwarf_file $srcfile5 $bad_buildid
+
+# Test where the separate debuginfo's buildid does not match, but then
+# we find a match in the .build-id directory.
+write_just_debugaltlink $srcfile6 ${binfile}7.o $ok_buildid2
+# Use 77 as the value so that if we load the bad debuginfo, we will
+# see the wrong result.
+write_dwarf_file $srcfile7 $bad_buildid 77
+write_dwarf_file $srcfile8 $ok_buildid2
+
+# Compile everything.
+for {set i 2} {$i <= 8} {incr i} {
+    if {[gdb_compile [standard_output_file [set srcfile$i]] \
+	     ${binfile}$i.o object nodebug] != ""} {
+	return -1
+    }
+}
+
+# Copy a file into the .build-id place for the "fallback" test.
+file copy -force -- ${binfile}8.o $basedir/$ok_prefix/$ok_suffix2.debug
+
+# Link the executables.
+if {[gdb_compile [list ${binfile}1.o ${binfile}2.o] ${binfile}-ok \
+	 executable {}] != ""} {
+    return -1
+}
+
+if {[gdb_compile [list ${binfile}1.o ${binfile}4.o] ${binfile}-mismatch \
+	 executable {}] != ""} {
+    return -1
+}
+
+if {[gdb_compile [list ${binfile}1.o ${binfile}6.o] ${binfile}-fallback \
+	 executable {}] != ""} {
+    return -1
+}
+
+
+foreach testname {ok mismatch fallback} {
+    with_test_prefix $testname {
+	gdb_exit
+	gdb_start
+	gdb_reinitialize_dir $srcdir/$subdir
+
+	gdb_test_no_output "set debug-file-directory $outdir"
+
+	gdb_load ${binfile}-${testname}
+
+	if {[runto_main]} {
+	    if {$testname == "mismatch"} {
+		gdb_test "print the_int" \
+		    "No symbol table is loaded.*"
+	    } else {
+		gdb_test "print the_int" " = 99"
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 5b19bb8..0f43e87 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -466,20 +466,27 @@ namespace eval Dwarf {
 	}
     }
 
-    proc _section {name} {
-	_emit "        .section $name"
+    proc _section {name {flags ""} {type ""}} {
+	if {$flags == "" && $type == ""} {
+	    _emit "        .section $name"
+	} elseif {$type == ""} {
+	    _emit "        .section $name, \"$flags\""
+	} else {
+	    _emit "        .section $name, \"$flags\", %$type"
+	}
     }
 
-    proc _defer_output {section body} {
+    # SECTION_SPEC is a list of arguments to _section.
+    proc _defer_output {section_spec body} {
 	variable _defer
 	variable _deferred_output
 
 	set old_defer $_defer
-	set _defer $section
+	set _defer [lindex $section_spec 0]
 
 	if {![info exists _deferred_output($_defer)]} {
 	    set _deferred_output($_defer) ""
-	    _section $section
+	    eval _section $section_spec
 	}
 
 	uplevel $body
@@ -869,6 +876,51 @@ namespace eval Dwarf {
 	unset the_array(_)
     }
 
+    # Emit a .gnu_debugaltlink section with the given file name and
+    # build-id.  The buildid should be represented as a hexadecimal
+    # string, like "ffeeddcc".
+    proc gnu_debugaltlink {filename buildid} {
+	_defer_output .gnu_debugaltlink {
+	    _op .ascii [_quote $filename]
+	    foreach {a b} [split $buildid {}] {
+		_op .byte 0x$a$b
+	    }
+	}
+    }
+
+    proc _note {type name hexdata} {
+	set namelen [expr [string length $name] + 1]
+
+	# Name size.
+	_op .4byte $namelen
+	# Data size.
+	_op .4byte [expr [string length $hexdata] / 2]
+	# Type.
+	_op .4byte $type
+	# The name.
+	_op .ascii [_quote $name]
+	# Alignment.
+	set align 2
+	set total [expr {($namelen + (1 << $align) - 1) & (-1 << $align)}]
+	for {set i $namelen} {$i < $total} {incr i} {
+	    _op .byte 0
+	}
+	# The data.
+	foreach {a b} [split $hexdata {}] {
+	    _op .byte 0x$a$b
+	}
+    }
+
+    # Emit a note section holding the given build-id.
+    proc build_id {buildid} {
+	_defer_output {.note.gnu.build-id a note} {
+	    # From elf/common.h.
+	    set NT_GNU_BUILD_ID 3
+
+	    _note $NT_GNU_BUILD_ID GNU $buildid
+	}
+    }
+
     # The top-level interface to the DWARF assembler.
     # FILENAME is the name of the file where the generated assembly
     # code is written.
-- 
1.8.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-08-08 17:37 [PATCH] fix PR symtab/15597 Tom Tromey
@ 2013-08-10  5:36 ` Alan Modra
  2013-08-12  8:41   ` Tom Tromey
  2013-08-14 11:25 ` nick clifton
  2013-10-08 19:38 ` Tom Tromey
  2 siblings, 1 reply; 14+ messages in thread
From: Alan Modra @ 2013-08-10  5:36 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, binutils

> Built and regtested on x86-64 Fedora 18.
> 
> 	* bfd-in2.h: Rebuild.
> 	* opncls.c (bfd_get_alt_debug_link_info): Add buildid_len

We don't have such a function in the FSF repo.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-08-10  5:36 ` Alan Modra
@ 2013-08-12  8:41   ` Tom Tromey
  2013-08-12  9:29     ` Alan Modra
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2013-08-12  8:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: binutils

>>>>> "Alan" == Alan Modra <amodra@gmail.com> writes:

>> Built and regtested on x86-64 Fedora 18.
>> 
>> * bfd-in2.h: Rebuild.
>> * opncls.c (bfd_get_alt_debug_link_info): Add buildid_len

Alan> We don't have such a function in the FSF repo.

It really is there.  Maybe you have a sticky tag or an old checkout or
something?

You can see it here:

http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/opncls.c.diff?r1=1.81&r2=1.82&cvsroot=src&f=h

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-08-12  8:41   ` Tom Tromey
@ 2013-08-12  9:29     ` Alan Modra
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Modra @ 2013-08-12  9:29 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, binutils

On Mon, Aug 12, 2013 at 02:41:38AM -0600, Tom Tromey wrote:
> >>>>> "Alan" == Alan Modra <amodra@gmail.com> writes:
> Alan> We don't have such a function in the FSF repo.
> 
> It really is there.  Maybe you have a sticky tag or an old checkout or
> something?

Huh, yes, it seems I had checked out an old version with -D.  I
normally do that sort of thing in a copy of the tree, to avoid exactly
this sort of problem.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-08-08 17:37 [PATCH] fix PR symtab/15597 Tom Tromey
  2013-08-10  5:36 ` Alan Modra
@ 2013-08-14 11:25 ` nick clifton
  2013-10-08 19:38 ` Tom Tromey
  2 siblings, 0 replies; 14+ messages in thread
From: nick clifton @ 2013-08-14 11:25 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches; +Cc: binutils

Hi Tom,

> This patch touches BFD.  Previously, gdb had its own code for parsing
> .gnu_debugaltlink; I changed it to use the BFD functions after those
> were introduced.  However, the BFD functions are incorrect -- they
> assume that .gnu_debugaltlink is formatted like .gnu_debuglink.
> However, it it is not.  Instead, it consists of a file name followed
> by the build-id -- no alignment, and the build-id is not a CRC.
>
> Fixing this properly is a bit of a pain.  But, because
> separate_alt_debug_file_exists just has a FIXME for the build-id case,
> I did not fix it properly.  Instead I introduced a hack.  This leaves
> BFD working just as well as it did before my patch.
>
> I'm willing to do something better here but I could use some guidance
> as to what.  It seems that the build-id code in BFD is largely punted
> on.

True - it would be nice to see this fixed, but it can be left for a 
future patch.


> 	* bfd-in2.h: Rebuild.
> 	* opncls.c (bfd_get_alt_debug_link_info): Add buildid_len
> 	parameter.  Change type of buildid_out.  Update.
> 	(get_alt_debug_link_info_shim): New function.
> 	(bfd_follow_gnu_debuglink): Use it.
>
> 	* Makefile.in (SFILES): Add build-id.c.
> 	(HFILES_NO_SRCDIR): Add build-id.h.
> 	* build-id.c: New file, largely from elfread.c.  Modified
> 	most functions.
> 	* build-id.h: New file.
> 	* dwarf2read.c (dwarf2_get_dwz_file): Update for change to
> 	bfd_get_alt_debug_link_info.  Verify dwz file's build-id.
> 	Search for dwz file using build-id.
> 	* elfread.c (build_id_bfd_get, build_id_verify)
> 	(build_id_to_debug_filename, find_separate_debug_file): Remove.
>

Approved - please apply.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-08-08 17:37 [PATCH] fix PR symtab/15597 Tom Tromey
  2013-08-10  5:36 ` Alan Modra
  2013-08-14 11:25 ` nick clifton
@ 2013-10-08 19:38 ` Tom Tromey
  2013-10-08 22:10   ` Hans-Peter Nilsson
  2013-10-10  0:17   ` [PATCH] fix PR symtab/15597 Doug Evans
  2 siblings, 2 replies; 14+ messages in thread
From: Tom Tromey @ 2013-10-08 19:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: binutils

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Tom> This patch fixes gdb PR symtab/15597.
Tom> The bug is that the .gnu_debugaltlink section includes the build-id of
Tom> the alt file, but gdb does not use it.

I'm checking in an updated version of this patch now.
The only change is to fix the patch for the objfile_name change in gdb.
I rebuilt and regtested this on x86-64 Fedora 18.

Tom

    	* bfd-in2.h: Rebuild.
    	* opncls.c (bfd_get_alt_debug_link_info): Add buildid_len
    	parameter.  Change type of buildid_out.  Update.
    	(get_alt_debug_link_info_shim): New function.
    	(bfd_follow_gnu_debuglink): Use it.
    
    	* Makefile.in (SFILES): Add build-id.c.
    	(HFILES_NO_SRCDIR): Add build-id.h.
    	* build-id.c: New file, largely from elfread.c.  Modified
    	most functions.
    	* build-id.h: New file.
    	* dwarf2read.c (dwarf2_get_dwz_file): Update for change to
    	bfd_get_alt_debug_link_info.  Verify dwz file's build-id.
    	Search for dwz file using build-id.
    	* elfread.c (build_id_bfd_get, build_id_verify)
    	(build_id_to_debug_filename, find_separate_debug_file): Remove.
    
    	* gdb.dwarf2/dwzbuildid.exp: New file.
    	* lib/dwarf.exp (Dwarf::_section): Add "flags" and "type"
    	parameters.
    	(Dwarf::_defer_output): Change "section" parameter to
    	"section_spec"; update.
    	(Dwarf::gnu_debugaltlink, Dwarf::_note, Dwarf::build_id): New
    	procs.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 41f7a68..67eb7da 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1067,7 +1067,8 @@ unsigned long bfd_calc_gnu_debuglink_crc32
 
 char *bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out);
 
-char *bfd_get_alt_debug_link_info (bfd *abfd, unsigned long *crc32_out);
+char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+    bfd_byte **buildid_out);
 
 char *bfd_follow_gnu_debuglink (bfd *abfd, const char *dir);
 
diff --git a/bfd/opncls.c b/bfd/opncls.c
index cd9c826..f29b2c8 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -1194,18 +1194,21 @@ FUNCTION
 	bfd_get_alt_debug_link_info
 
 SYNOPSIS
-	char *bfd_get_alt_debug_link_info (bfd *abfd, unsigned long *crc32_out);
+	char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+			                   bfd_byte **buildid_out);
 
 DESCRIPTION
 	Fetch the filename and BuildID value for any alternate debuginfo
 	associated with @var{abfd}.  Return NULL if no such info found,
-	otherwise return filename and update @var{buildid_out}.  The
-	returned filename is allocated with @code{malloc}; freeing it
-	is the responsibility of the caller.
+	otherwise return filename and update @var{buildid_len} and
+	@var{buildid_out}.  The returned filename and build_id are
+	allocated with @code{malloc}; freeing them is the
+	responsibility of the caller.
 */
 
 char *
-bfd_get_alt_debug_link_info (bfd * abfd, unsigned long * buildid_out)
+bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+			     bfd_byte **buildid_out)
 {
   asection *sect;
   bfd_byte *contents;
@@ -1213,6 +1216,7 @@ bfd_get_alt_debug_link_info (bfd * abfd, unsigned long * buildid_out)
   char *name;
 
   BFD_ASSERT (abfd);
+  BFD_ASSERT (buildid_len);
   BFD_ASSERT (buildid_out);
 
   sect = bfd_get_section_by_name (abfd, GNU_DEBUGALTLINK);
@@ -1227,12 +1231,13 @@ bfd_get_alt_debug_link_info (bfd * abfd, unsigned long * buildid_out)
       return NULL;
     }
 
-  /* BuildID value is stored after the filename, aligned up to 4 bytes.  */
+  /* BuildID value is stored after the filename.  */
   name = (char *) contents;
   buildid_offset = strlen (name) + 1;
-  buildid_offset = (buildid_offset + 3) & ~3;
 
-  * buildid_out = bfd_get_32 (abfd, contents + buildid_offset);
+  *buildid_len = bfd_get_section_size (sect) - buildid_offset;
+  *buildid_out = bfd_malloc (*buildid_len);
+  memcpy (*buildid_out, contents + buildid_offset, *buildid_len);
 
   return name;
 }
@@ -1466,6 +1471,24 @@ bfd_follow_gnu_debuglink (bfd *abfd, const char *dir)
 				   separate_debug_file_exists);
 }
 
+/* Helper for bfd_follow_gnu_debugaltlink.  It just pretends to return
+   a CRC.  .gnu_debugaltlink supplies a build-id, which is different,
+   but this is ok because separate_alt_debug_file_exists ignores the
+   CRC anyway.  */
+
+static char *
+get_alt_debug_link_info_shim (bfd * abfd, unsigned long *crc32_out)
+{
+  size_t len;
+  bfd_byte *buildid = NULL;
+  char *result = bfd_get_alt_debug_link_info (abfd, &len, &buildid);
+
+  *crc32_out = 0;
+  free (buildid);
+
+  return result;
+}
+
 /*
 FUNCTION
 	bfd_follow_gnu_debugaltlink
@@ -1496,7 +1519,7 @@ char *
 bfd_follow_gnu_debugaltlink (bfd *abfd, const char *dir)
 {
   return find_separate_debug_file (abfd, dir,
-				   bfd_get_alt_debug_link_info,
+				   get_alt_debug_link_info_shim,
 				   separate_alt_debug_file_exists);
 }
 
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 6b8927a..2aa8134 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -717,7 +717,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	bfd-target.c \
 	block.c blockframe.c \
 	breakpoint.c break-catch-sig.c break-catch-throw.c \
-	buildsym.c \
+	build-id.c buildsym.c \
 	c-exp.y c-lang.c c-typeprint.c c-valprint.c \
 	charset.c cleanups.c cli-out.c coffread.c coff-pe-read.c \
 	complaints.c completer.c continuations.c corefile.c corelow.c \
@@ -814,7 +814,8 @@ tui/tui-file.h tui/tui-command.h tui/tui-disasm.h tui/tui-wingeneral.h \
 tui/tui-windata.h tui/tui-data.h tui/tui-win.h tui/tui-stack.h \
 tui/tui-winsource.h tui/tui-regs.h tui/tui-io.h tui/tui-layout.h \
 tui/tui-source.h sol2-tdep.h gregset.h sh-tdep.h sh64-tdep.h \
-expression.h score-tdep.h gdb_select.h ser-tcp.h buildsym.h valprint.h \
+expression.h score-tdep.h gdb_select.h ser-tcp.h \
+build-id.h buildsym.h valprint.h \
 typeprint.h mi/mi-getopt.h mi/mi-parse.h mi/mi-console.h \
 mi/mi-out.h mi/mi-main.h mi/mi-common.h mi/mi-cmds.h linux-nat.h \
 complaints.h gdb_proc_service.h gdb_regex.h xtensa-tdep.h inf-loop.h \
@@ -911,7 +912,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	event-loop.o event-top.o inf-loop.o completer.o \
 	gdbarch.o arch-utils.o gdbtypes.o gdb_bfd.o gdb_obstack.o \
 	osabi.o copying.o \
-	memattr.o mem-break.o target.o parse.o language.o buildsym.o \
+	memattr.o mem-break.o target.o parse.o language.o \
+	build-id.o buildsym.o \
 	findcmd.o \
 	std-regs.o \
 	signals.o \
diff --git a/gdb/build-id.c b/gdb/build-id.c
new file mode 100644
index 0000000..330ed18
--- /dev/null
+++ b/gdb/build-id.c
@@ -0,0 +1,168 @@
+/* build-id-related functions.
+
+   Copyright (C) 1991-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "bfd.h"
+#include "elf-bfd.h"
+#include "gdb_bfd.h"
+#include "build-id.h"
+#include "gdb_string.h"
+#include "gdb_vecs.h"
+#include "symfile.h"
+#include "objfiles.h"
+#include "filenames.h"
+
+/* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
+
+static const struct elf_build_id *
+build_id_bfd_get (bfd *abfd)
+{
+  if (!bfd_check_format (abfd, bfd_object)
+      || bfd_get_flavour (abfd) != bfd_target_elf_flavour
+      /* Although this is ELF_specific, it is safe to do in generic
+	 code because it does not rely on any ELF-specific symbols at
+	 link time, and if the ELF code is not available in BFD, then
+	 ABFD will not have the ELF flavour.  */
+      || elf_tdata (abfd)->build_id == NULL)
+    return NULL;
+
+  return elf_tdata (abfd)->build_id;
+}
+
+/* See build-id.h.  */
+
+int
+build_id_verify (bfd *abfd, size_t check_len, const bfd_byte *check)
+{
+  const struct elf_build_id *found;
+  int retval = 0;
+
+  found = build_id_bfd_get (abfd);
+
+  if (found == NULL)
+    warning (_("File \"%s\" has no build-id, file skipped"),
+	     bfd_get_filename (abfd));
+  else if (found->size != check_len
+           || memcmp (found->data, check, found->size) != 0)
+    warning (_("File \"%s\" has a different build-id, file skipped"),
+	     bfd_get_filename (abfd));
+  else
+    retval = 1;
+
+  return retval;
+}
+
+/* See build-id.h.  */
+
+bfd *
+build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id)
+{
+  char *link, *debugdir;
+  VEC (char_ptr) *debugdir_vec;
+  struct cleanup *back_to;
+  int ix;
+  bfd *abfd = NULL;
+
+  /* DEBUG_FILE_DIRECTORY/.build-id/ab/cdef */
+  link = alloca (strlen (debug_file_directory) + (sizeof "/.build-id/" - 1) + 1
+		 + 2 * build_id_len + (sizeof ".debug" - 1) + 1);
+
+  /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
+     cause "/.build-id/..." lookups.  */
+
+  debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
+  back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
+
+  for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
+    {
+      size_t debugdir_len = strlen (debugdir);
+      const gdb_byte *data = build_id;
+      size_t size = build_id_len;
+      char *s;
+      char *filename = NULL;
+
+      memcpy (link, debugdir, debugdir_len);
+      s = &link[debugdir_len];
+      s += sprintf (s, "/.build-id/");
+      if (size > 0)
+	{
+	  size--;
+	  s += sprintf (s, "%02x", (unsigned) *data++);
+	}
+      if (size > 0)
+	*s++ = '/';
+      while (size-- > 0)
+	s += sprintf (s, "%02x", (unsigned) *data++);
+      strcpy (s, ".debug");
+
+      /* lrealpath() is expensive even for the usually non-existent files.  */
+      if (access (link, F_OK) == 0)
+	filename = lrealpath (link);
+
+      if (filename == NULL)
+	continue;
+
+      /* We expect to be silent on the non-existing files.  */
+      abfd = gdb_bfd_open_maybe_remote (filename);
+      if (abfd == NULL)
+	continue;
+
+      if (build_id_verify (abfd, build_id_len, build_id))
+	break;
+
+      gdb_bfd_unref (abfd);
+      abfd = NULL;
+    }
+
+  do_cleanups (back_to);
+  return abfd;
+}
+
+/* See build-id.h.  */
+
+char *
+find_separate_debug_file_by_buildid (struct objfile *objfile)
+{
+  const struct elf_build_id *build_id;
+
+  build_id = build_id_bfd_get (objfile->obfd);
+  if (build_id != NULL)
+    {
+      bfd *abfd;
+
+      abfd = build_id_to_debug_bfd (build_id->size, build_id->data);
+      /* Prevent looping on a stripped .debug file.  */
+      if (abfd != NULL
+	  && filename_cmp (bfd_get_filename (abfd),
+			   objfile_name (objfile)) == 0)
+        {
+	  warning (_("\"%s\": separate debug info file has no debug info"),
+		   bfd_get_filename (abfd));
+	  gdb_bfd_unref (abfd);
+	}
+      else if (abfd != NULL)
+	{
+	  char *result = xstrdup (bfd_get_filename (abfd));
+
+	  gdb_bfd_unref (abfd);
+	  return result;
+	}
+    }
+  return NULL;
+}
diff --git a/gdb/build-id.h b/gdb/build-id.h
new file mode 100644
index 0000000..b3ea4fb
--- /dev/null
+++ b/gdb/build-id.h
@@ -0,0 +1,44 @@
+/* build-id-related functions.
+
+   Copyright (C) 1991-2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef BUILD_ID_H
+#define BUILD_ID_H
+
+/* Return true if ABFD has NT_GNU_BUILD_ID matching the CHECK value.
+   Otherwise, issue a warning and return false.  */
+
+extern int build_id_verify (bfd *abfd,
+			    size_t check_len, const bfd_byte *check);
+
+
+/* Find and open a BFD given a build-id.  If no BFD can be found,
+   return NULL.  The returned reference to the BFD must be released by
+   the caller.  */
+
+extern bfd *build_id_to_debug_bfd (size_t build_id_len,
+				   const bfd_byte *build_id);
+
+/* Find the separate debug file for OBJFILE, by using the build-id
+   associated with OBJFILE's BFD.  If successful, returns a malloc'd
+   file name for the separate debug file.  The caller must free this.
+   Otherwise, returns NULL.  */
+
+extern char *find_separate_debug_file_by_buildid (struct objfile *objfile);
+
+#endif /* BUILD_ID_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 5e2ace7..4cb66db 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -69,6 +69,7 @@
 #include "f-lang.h"
 #include "source.h"
 #include "filestuff.h"
+#include "build-id.h"
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -2364,14 +2365,15 @@ dwarf2_get_dwz_file (void)
   struct cleanup *cleanup;
   const char *filename;
   struct dwz_file *result;
-  unsigned long buildid;
+  size_t buildid_len;
+  bfd_byte *buildid;
 
   if (dwarf2_per_objfile->dwz_file != NULL)
     return dwarf2_per_objfile->dwz_file;
 
   bfd_set_error (bfd_error_no_error);
   data = bfd_get_alt_debug_link_info (dwarf2_per_objfile->objfile->obfd,
-				      &buildid);
+				      &buildid_len, &buildid);
   if (data == NULL)
     {
       if (bfd_get_error () == bfd_error_no_error)
@@ -2380,6 +2382,7 @@ dwarf2_get_dwz_file (void)
 	     bfd_errmsg (bfd_get_error ()));
     }
   cleanup = make_cleanup (xfree, data);
+  make_cleanup (xfree, buildid);
 
   filename = (const char *) data;
   if (!IS_ABSOLUTE_PATH (filename))
@@ -2396,20 +2399,25 @@ dwarf2_get_dwz_file (void)
       filename = rel;
     }
 
-  /* The format is just a NUL-terminated file name, followed by the
-     build-id.  For now, though, we ignore the build-id.  */
+  /* First try the file name given in the section.  If that doesn't
+     work, try to use the build-id instead.  */
   dwz_bfd = gdb_bfd_open (filename, gnutarget, -1);
-  if (dwz_bfd == NULL)
-    error (_("could not read '%s': %s"), filename,
-	   bfd_errmsg (bfd_get_error ()));
-
-  if (!bfd_check_format (dwz_bfd, bfd_object))
+  if (dwz_bfd != NULL)
     {
-      gdb_bfd_unref (dwz_bfd);
-      error (_("file '%s' was not usable: %s"), filename,
-	     bfd_errmsg (bfd_get_error ()));
+      if (!build_id_verify (dwz_bfd, buildid_len, buildid))
+	{
+	  gdb_bfd_unref (dwz_bfd);
+	  dwz_bfd = NULL;
+	}
     }
 
+  if (dwz_bfd == NULL)
+    dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
+
+  if (dwz_bfd == NULL)
+    error (_("could not find '.gnu_debugaltlink' file for %s"),
+	   objfile_name (dwarf2_per_objfile->objfile));
+
   result = OBSTACK_ZALLOC (&dwarf2_per_objfile->objfile->objfile_obstack,
 			   struct dwz_file);
   result->dwz_bfd = dwz_bfd;
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 4a3d713..0095ad9 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -45,6 +45,7 @@
 #include "regcache.h"
 #include "bcache.h"
 #include "gdb_bfd.h"
+#include "build-id.h"
 
 extern void _initialize_elfread (void);
 
@@ -1087,131 +1088,6 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b)
   update_breakpoint_locations (b, sals, sals_end);
 }
 
-/* Locate NT_GNU_BUILD_ID from ABFD and return its content.  */
-
-static const struct elf_build_id *
-build_id_bfd_get (bfd *abfd)
-{
-  if (!bfd_check_format (abfd, bfd_object)
-      || bfd_get_flavour (abfd) != bfd_target_elf_flavour
-      || elf_tdata (abfd)->build_id == NULL)
-    return NULL;
-
-  return elf_tdata (abfd)->build_id;
-}
-
-/* Return if FILENAME has NT_GNU_BUILD_ID matching the CHECK value.  */
-
-static int
-build_id_verify (const char *filename, const struct elf_build_id *check)
-{
-  bfd *abfd;
-  const struct elf_build_id *found;
-  int retval = 0;
-
-  /* We expect to be silent on the non-existing files.  */
-  abfd = gdb_bfd_open_maybe_remote (filename);
-  if (abfd == NULL)
-    return 0;
-
-  found = build_id_bfd_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;
-
-  gdb_bfd_unref (abfd);
-
-  return retval;
-}
-
-static char *
-build_id_to_debug_filename (const struct elf_build_id *build_id)
-{
-  char *link, *debugdir, *retval = NULL;
-  VEC (char_ptr) *debugdir_vec;
-  struct cleanup *back_to;
-  int ix;
-
-  /* DEBUG_FILE_DIRECTORY/.build-id/ab/cdef */
-  link = alloca (strlen (debug_file_directory) + (sizeof "/.build-id/" - 1) + 1
-		 + 2 * build_id->size + (sizeof ".debug" - 1) + 1);
-
-  /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
-     cause "/.build-id/..." lookups.  */
-
-  debugdir_vec = dirnames_to_char_ptr_vec (debug_file_directory);
-  back_to = make_cleanup_free_char_ptr_vec (debugdir_vec);
-
-  for (ix = 0; VEC_iterate (char_ptr, debugdir_vec, ix, debugdir); ++ix)
-    {
-      size_t debugdir_len = strlen (debugdir);
-      const gdb_byte *data = build_id->data;
-      size_t size = build_id->size;
-      char *s;
-
-      memcpy (link, debugdir, debugdir_len);
-      s = &link[debugdir_len];
-      s += sprintf (s, "/.build-id/");
-      if (size > 0)
-	{
-	  size--;
-	  s += sprintf (s, "%02x", (unsigned) *data++);
-	}
-      if (size > 0)
-	*s++ = '/';
-      while (size-- > 0)
-	s += sprintf (s, "%02x", (unsigned) *data++);
-      strcpy (s, ".debug");
-
-      /* lrealpath() is expensive even for the usually non-existent files.  */
-      if (access (link, F_OK) == 0)
-	retval = lrealpath (link);
-
-      if (retval != NULL && !build_id_verify (retval, build_id))
-	{
-	  xfree (retval);
-	  retval = NULL;
-	}
-
-      if (retval != NULL)
-	break;
-    }
-
-  do_cleanups (back_to);
-  return retval;
-}
-
-static char *
-find_separate_debug_file_by_buildid (struct objfile *objfile)
-{
-  const struct elf_build_id *build_id;
-
-  build_id = build_id_bfd_get (objfile->obfd);
-  if (build_id != NULL)
-    {
-      char *build_id_name;
-
-      build_id_name = build_id_to_debug_filename (build_id);
-      /* Prevent looping on a stripped .debug file.  */
-      if (build_id_name != NULL
-	  && filename_cmp (build_id_name, objfile_name (objfile)) == 0)
-        {
-	  warning (_("\"%s\": separate debug info file has no debug info"),
-		   build_id_name);
-	  xfree (build_id_name);
-	}
-      else if (build_id_name != NULL)
-        return build_id_name;
-    }
-  return NULL;
-}
-
 /* Scan and build partial symbols for a symbol file.
    We have been initialized by a call to elf_symfile_init, which
    currently does nothing.
diff --git a/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp b/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
new file mode 100644
index 0000000..e6ae881
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
@@ -0,0 +1,170 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+# No remote host testing either.
+if {[is_remote host]} {
+    return 0
+}
+
+
+# Lots of source files since we test a few cases and make new files
+# for each.
+# The tests are:
+#     ok - the main file refers to a dwz and the buildids match
+#     mismatch - the buildids do not match
+#     fallback - the buildids do not match but a match is found via buildid
+standard_testfile main.c \
+    dwzbuildid-ok-base.S dwzbuildid-ok-sep.S \
+    dwzbuildid-mismatch-base.S dwzbuildid-mismatch-sep.S \
+    dwzbuildid-fallback-base.S dwzbuildid-fallback-sep.S \
+    dwzbuildid-fallback-ok.S
+    
+# Write some assembly that just has a .gnu_debugaltlink section.
+proc write_just_debugaltlink {filename dwzname buildid} {
+    set asm_file [standard_output_file $filename]
+
+    Dwarf::assemble $asm_file {
+	upvar dwzname dwzname
+	upvar buildid buildid
+
+	gnu_debugaltlink $dwzname $buildid
+
+	# Only the DWARF reader checks .gnu_debugaltlink, so make sure
+	# there is a bit of DWARF in here.
+	cu {} {
+	    compile_unit {{language @DW_LANG_C}} {
+	    }
+	}
+    }
+}
+
+# Write some DWARF that also sets the buildid.
+proc write_dwarf_file {filename buildid {value 99}} {
+    set asm_file [standard_output_file $filename]
+
+    Dwarf::assemble $asm_file {
+	declare_labels partial_label double_label int_label int_label2
+
+	upvar buildid buildid
+	upvar value value
+
+	build_id $buildid
+
+	cu {} {
+	    compile_unit {{language @DW_LANG_C}} {
+		int_label2: base_type {
+		    {name int}
+		    {byte_size 4 sdata}
+		    {encoding @DW_ATE_signed}
+		}
+
+		constant {
+		    {name the_int}
+		    {type :$int_label2}
+		    {const_value $value data1}
+		}
+	    }
+	}
+    }
+}
+
+if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
+	   object {nodebug}] != "" } {
+    return -1
+}
+
+# The values don't really matter, just whether they are equal.
+set ok_prefix 01
+set ok_suffix 0203040506
+set ok_suffix2 02030405ff
+set ok_buildid ${ok_prefix}${ok_suffix}
+set ok_buildid2 ${ok_prefix}${ok_suffix2}
+set bad_buildid ffffffffffff
+
+set outdir [standard_output_file {}]
+set basedir $outdir/.build-id
+file mkdir $basedir $basedir/$ok_prefix
+
+# Test where the separate debuginfo's buildid matches.
+write_just_debugaltlink $srcfile2 ${binfile}3.o $ok_buildid
+write_dwarf_file $srcfile3 $ok_buildid
+
+# Test where the separate debuginfo's buildid does not match.
+write_just_debugaltlink $srcfile4 ${binfile}5.o $ok_buildid
+write_dwarf_file $srcfile5 $bad_buildid
+
+# Test where the separate debuginfo's buildid does not match, but then
+# we find a match in the .build-id directory.
+write_just_debugaltlink $srcfile6 ${binfile}7.o $ok_buildid2
+# Use 77 as the value so that if we load the bad debuginfo, we will
+# see the wrong result.
+write_dwarf_file $srcfile7 $bad_buildid 77
+write_dwarf_file $srcfile8 $ok_buildid2
+
+# Compile everything.
+for {set i 2} {$i <= 8} {incr i} {
+    if {[gdb_compile [standard_output_file [set srcfile$i]] \
+	     ${binfile}$i.o object nodebug] != ""} {
+	return -1
+    }
+}
+
+# Copy a file into the .build-id place for the "fallback" test.
+file copy -force -- ${binfile}8.o $basedir/$ok_prefix/$ok_suffix2.debug
+
+# Link the executables.
+if {[gdb_compile [list ${binfile}1.o ${binfile}2.o] ${binfile}-ok \
+	 executable {}] != ""} {
+    return -1
+}
+
+if {[gdb_compile [list ${binfile}1.o ${binfile}4.o] ${binfile}-mismatch \
+	 executable {}] != ""} {
+    return -1
+}
+
+if {[gdb_compile [list ${binfile}1.o ${binfile}6.o] ${binfile}-fallback \
+	 executable {}] != ""} {
+    return -1
+}
+
+
+foreach testname {ok mismatch fallback} {
+    with_test_prefix $testname {
+	gdb_exit
+	gdb_start
+	gdb_reinitialize_dir $srcdir/$subdir
+
+	gdb_test_no_output "set debug-file-directory $outdir"
+
+	gdb_load ${binfile}-${testname}
+
+	if {[runto_main]} {
+	    if {$testname == "mismatch"} {
+		gdb_test "print the_int" \
+		    "No symbol table is loaded.*"
+	    } else {
+		gdb_test "print the_int" " = 99"
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 5264def..c28b986 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -523,20 +523,27 @@ namespace eval Dwarf {
 	}
     }
 
-    proc _section {name} {
-	_emit "        .section $name"
+    proc _section {name {flags ""} {type ""}} {
+	if {$flags == "" && $type == ""} {
+	    _emit "        .section $name"
+	} elseif {$type == ""} {
+	    _emit "        .section $name, \"$flags\""
+	} else {
+	    _emit "        .section $name, \"$flags\", %$type"
+	}
     }
 
-    proc _defer_output {section body} {
+    # SECTION_SPEC is a list of arguments to _section.
+    proc _defer_output {section_spec body} {
 	variable _defer
 	variable _deferred_output
 
 	set old_defer $_defer
-	set _defer $section
+	set _defer [lindex $section_spec 0]
 
 	if {![info exists _deferred_output($_defer)]} {
 	    set _deferred_output($_defer) ""
-	    _section $section
+	    eval _section $section_spec
 	}
 
 	uplevel $body
@@ -948,6 +955,51 @@ namespace eval Dwarf {
 	unset the_array(_)
     }
 
+    # Emit a .gnu_debugaltlink section with the given file name and
+    # build-id.  The buildid should be represented as a hexadecimal
+    # string, like "ffeeddcc".
+    proc gnu_debugaltlink {filename buildid} {
+	_defer_output .gnu_debugaltlink {
+	    _op .ascii [_quote $filename]
+	    foreach {a b} [split $buildid {}] {
+		_op .byte 0x$a$b
+	    }
+	}
+    }
+
+    proc _note {type name hexdata} {
+	set namelen [expr [string length $name] + 1]
+
+	# Name size.
+	_op .4byte $namelen
+	# Data size.
+	_op .4byte [expr [string length $hexdata] / 2]
+	# Type.
+	_op .4byte $type
+	# The name.
+	_op .ascii [_quote $name]
+	# Alignment.
+	set align 2
+	set total [expr {($namelen + (1 << $align) - 1) & (-1 << $align)}]
+	for {set i $namelen} {$i < $total} {incr i} {
+	    _op .byte 0
+	}
+	# The data.
+	foreach {a b} [split $hexdata {}] {
+	    _op .byte 0x$a$b
+	}
+    }
+
+    # Emit a note section holding the given build-id.
+    proc build_id {buildid} {
+	_defer_output {.note.gnu.build-id a note} {
+	    # From elf/common.h.
+	    set NT_GNU_BUILD_ID 3
+
+	    _note $NT_GNU_BUILD_ID GNU $buildid
+	}
+    }
+
     # The top-level interface to the DWARF assembler.
     # FILENAME is the name of the file where the generated assembly
     # code is written.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-10-08 19:38 ` Tom Tromey
@ 2013-10-08 22:10   ` Hans-Peter Nilsson
  2013-10-09  1:27     ` Tom Tromey
  2013-10-10  0:17   ` [PATCH] fix PR symtab/15597 Doug Evans
  1 sibling, 1 reply; 14+ messages in thread
From: Hans-Peter Nilsson @ 2013-10-08 22:10 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, binutils

On Tue, 8 Oct 2013, Tom Tromey wrote:
> >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
>
> Tom> This patch fixes gdb PR symtab/15597.
> Tom> The bug is that the .gnu_debugaltlink section includes the build-id of
> Tom> the alt file, but gdb does not use it.
>
> I'm checking in an updated version of this patch now.
> The only change is to fix the patch for the objfile_name change in gdb.
> I rebuilt and regtested this on x86-64 Fedora 18.
>
> Tom
>
>     	* bfd-in2.h: Rebuild.

> diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
> index 41f7a68..67eb7da 100644
> --- a/bfd/bfd-in2.h
> +++ b/bfd/bfd-in2.h
> @@ -1067,7 +1067,8 @@ unsigned long bfd_calc_gnu_debuglink_crc32
>
>  char *bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out);
>
> -char *bfd_get_alt_debug_link_info (bfd *abfd, unsigned long *crc32_out);
> +char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
> +    bfd_byte **buildid_out);

Sorry, but this broke many simulator builds; it's the first
size_t in bfd-in2.h and neither it nor all its clients include
stddef.h.  Could you consider using bfd_size_type instead?

Example, frv-elf:
...
gcc -DHAVE_CONFIG_H   -DWITH_DEFAULT_MODEL='"fr500"'  -DPROFILE=1 -DWITH_PROFILE=-1   -DWITH_ALIGNMENT=STRICT_ALIGNMENT  -DWITH_TARGET_BYTE_ORDER=BIG_ENDIAN -DWITH_ENVIRONMENT=ALL_ENVIRONMENT  -DWITH_HW=1 -DWITH_HOST_BYTE_ORDER=LITTLE_ENDIAN -DDEFAULT_INLINE=0    -DWITH_SCACHE=16384          -I. -I/tmp/hpautotest-sim/src/sim/frv -I../common -I/tmp/hpautotest-sim/src/sim/frv/../common -I../../include -I/tmp/hpautotest-sim/src/sim/frv/../../include -I../../bfd -I/tmp/hpautotest-sim/src/sim/frv/../../bfd -I../../opcodes -I/tmp/hpautotest-sim/src/sim/frv/../../opcodes  -g -O2 -c -o cgen-utils.o -MT cgen-utils.o -MMD -MP -MF .deps/cgen-utils.Tpo /tmp/hpautotest-sim/src/sim/frv/../common/cgen-utils.c
In file included from /tmp/hpautotest-sim/src/sim/frv/../common/cgen-utils.c:21:0:
../../bfd/bfd.h:1070:48: error: unknown type name 'size_t'
make[3]: Leaving directory `/tmp/hpautotest-sim/frv-elf/sim/frv'
make[3]: *** [cgen-utils.o] Error 1
make[2]: Leaving directory `/tmp/hpautotest-sim/frv-elf/sim'
make[2]: *** [all] Error 1

Others (not exhaustive):
cris-elf, h8300-elf, iq2000-elf, m32r-elf, mips-elf, mn10300-elf

brgds, H-P
PS: how about that src-release patch?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-10-08 22:10   ` Hans-Peter Nilsson
@ 2013-10-09  1:27     ` Tom Tromey
  2013-10-09 14:09       ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2013-10-09  1:27 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gdb-patches, binutils

>>>>> "H-P" == Hans-Peter Nilsson <hp@bitrange.com> writes:

H-P> Sorry, but this broke many simulator builds; it's the first
H-P> size_t in bfd-in2.h and neither it nor all its clients include
H-P> stddef.h.  Could you consider using bfd_size_type instead?

Sorry about that.
I'll fix it in the morning.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-10-09  1:27     ` Tom Tromey
@ 2013-10-09 14:09       ` Tom Tromey
  2013-10-09 14:54         ` Build regression on 32-bit hosts [Re: [PATCH] fix PR symtab/15597] Jan Kratochvil
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2013-10-09 14:09 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gdb-patches, binutils

Tom> Sorry about that.
Tom> I'll fix it in the morning.

Here's the fix I am checking in.

Tom

bfd/ChangeLog:
2013-10-09  Tom Tromey  <tromey@redhat.com>

	* bfd-in2.h: Rebuild.
	* opncls.c (bfd_get_alt_debug_link_info): Change type of
	buildid_len to bfd_size_type.

gdb/ChangeLog:
2013-10-09  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_get_dwz_file): Update for type change in
	bfd_get_alt_debug_link_info.

Index: bfd/bfd-in2.h
===================================================================
RCS file: /cvs/src/src/bfd/bfd-in2.h,v
retrieving revision 1.613
diff -u -r1.613 bfd-in2.h
--- bfd/bfd-in2.h	8 Oct 2013 19:56:14 -0000	1.613
+++ bfd/bfd-in2.h	9 Oct 2013 14:01:57 -0000
@@ -1067,7 +1067,8 @@
 
 char *bfd_get_debug_link_info (bfd *abfd, unsigned long *crc32_out);
 
-char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+char *bfd_get_alt_debug_link_info (bfd * abfd,
+    bfd_size_type *buildid_len,
     bfd_byte **buildid_out);
 
 char *bfd_follow_gnu_debuglink (bfd *abfd, const char *dir);
Index: bfd/opncls.c
===================================================================
RCS file: /cvs/src/src/bfd/opncls.c,v
retrieving revision 1.83
diff -u -r1.83 opncls.c
--- bfd/opncls.c	8 Oct 2013 19:56:14 -0000	1.83
+++ bfd/opncls.c	9 Oct 2013 14:01:58 -0000
@@ -1194,7 +1194,8 @@
 	bfd_get_alt_debug_link_info
 
 SYNOPSIS
-	char *bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+	char *bfd_get_alt_debug_link_info (bfd * abfd,
+					   bfd_size_type *buildid_len,
 			                   bfd_byte **buildid_out);
 
 DESCRIPTION
@@ -1207,7 +1208,7 @@
 */
 
 char *
-bfd_get_alt_debug_link_info (bfd * abfd, size_t *buildid_len,
+bfd_get_alt_debug_link_info (bfd * abfd, bfd_size_type *buildid_len,
 			     bfd_byte **buildid_out)
 {
   asection *sect;
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.842
diff -u -r1.842 dwarf2read.c
--- gdb/dwarf2read.c	8 Oct 2013 19:56:15 -0000	1.842
+++ gdb/dwarf2read.c	9 Oct 2013 14:02:01 -0000
@@ -2365,6 +2365,7 @@
   struct cleanup *cleanup;
   const char *filename;
   struct dwz_file *result;
+  bfd_size_type buildid_len_arg;
   size_t buildid_len;
   bfd_byte *buildid;
 
@@ -2373,7 +2374,7 @@
 
   bfd_set_error (bfd_error_no_error);
   data = bfd_get_alt_debug_link_info (dwarf2_per_objfile->objfile->obfd,
-				      &buildid_len, &buildid);
+				      &buildid_len_arg, &buildid);
   if (data == NULL)
     {
       if (bfd_get_error () == bfd_error_no_error)
@@ -2384,6 +2385,8 @@
   cleanup = make_cleanup (xfree, data);
   make_cleanup (xfree, buildid);
 
+  buildid_len = (size_t) buildid_len_arg;
+
   filename = (const char *) data;
   if (!IS_ABSOLUTE_PATH (filename))
     {

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Build regression on 32-bit hosts  [Re: [PATCH] fix PR symtab/15597]
  2013-10-09 14:09       ` Tom Tromey
@ 2013-10-09 14:54         ` Jan Kratochvil
  2013-10-09 15:46           ` Tom Tromey
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kratochvil @ 2013-10-09 14:54 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Hans-Peter Nilsson, gdb-patches, binutils

On Wed, 09 Oct 2013 16:08:56 +0200, Tom Tromey wrote:
> Here's the fix I am checking in.

commit 1677499f5afa31d6f07f0caca3eaca3855e70348
Author: Tom Tromey <tromey@redhat.com>
Date:   Wed Oct 9 14:26:26 2013 +0000
    bfd
        * bfd-in2.h: Rebuild.
        * opncls.c (bfd_get_alt_debug_link_info): Change type of
        buildid_len to bfd_size_type.
    gdb
        * dwarf2read.c (dwarf2_get_dwz_file): Update for type change in
        bfd_get_alt_debug_link_info.

opncls.c: In function ‘get_alt_debug_link_info_shim’:
opncls.c:1485:3: error: passing argument 2 of ‘bfd_get_alt_debug_link_info’ from incompatible pointer type [-Werror]
opncls.c:1211:1: note: expected ‘bfd_size_type *’ but argument is of type ‘size_t *’


Jan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Build regression on 32-bit hosts  [Re: [PATCH] fix PR symtab/15597]
  2013-10-09 14:54         ` Build regression on 32-bit hosts [Re: [PATCH] fix PR symtab/15597] Jan Kratochvil
@ 2013-10-09 15:46           ` Tom Tromey
  0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2013-10-09 15:46 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Hans-Peter Nilsson, gdb-patches, binutils

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> opncls.c: In function ‘get_alt_debug_link_info_shim’:
Jan> opncls.c:1485:3: error: passing argument 2 of ‘bfd_get_alt_debug_link_info’ from incompatible pointer type [-Werror]
Jan> opncls.c:1211:1: note: expected ‘bfd_size_type *’ but argument is of type ‘size_t *’

I see my BFD record still stands.

Here's a follow-up.

Tom

2013-10-09  Tom Tromey  <tromey@redhat.com>

	* opncls.c (get_alt_debug_link_info_shim): Update type of 'len'.

Index: opncls.c
===================================================================
RCS file: /cvs/src/src/bfd/opncls.c,v
retrieving revision 1.84
diff -u -r1.84 opncls.c
--- opncls.c	9 Oct 2013 14:26:26 -0000	1.84
+++ opncls.c	9 Oct 2013 15:45:36 -0000
@@ -1480,7 +1480,7 @@
 static char *
 get_alt_debug_link_info_shim (bfd * abfd, unsigned long *crc32_out)
 {
-  size_t len;
+  bfd_size_type len;
   bfd_byte *buildid = NULL;
   char *result = bfd_get_alt_debug_link_info (abfd, &len, &buildid);
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-10-08 19:38 ` Tom Tromey
  2013-10-08 22:10   ` Hans-Peter Nilsson
@ 2013-10-10  0:17   ` Doug Evans
  2013-10-10  2:29     ` Tom Tromey
  1 sibling, 1 reply; 14+ messages in thread
From: Doug Evans @ 2013-10-10  0:17 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Tom Tromey writes:
 > >>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:
 > 
 > Tom> This patch fixes gdb PR symtab/15597.
 > Tom> The bug is that the .gnu_debugaltlink section includes the build-id of
 > Tom> the alt file, but gdb does not use it.
 > 
 > I'm checking in an updated version of this patch now.
 > The only change is to fix the patch for the objfile_name change in gdb.
 > I rebuilt and regtested this on x86-64 Fedora 18.
 > 
 > Tom
 > 
 >     	* bfd-in2.h: Rebuild.
 >     	* opncls.c (bfd_get_alt_debug_link_info): Add buildid_len
 >     	parameter.  Change type of buildid_out.  Update.
 >     	(get_alt_debug_link_info_shim): New function.
 >     	(bfd_follow_gnu_debuglink): Use it.
 >     
 >     	* Makefile.in (SFILES): Add build-id.c.
 >     	(HFILES_NO_SRCDIR): Add build-id.h.
 >     	* build-id.c: New file, largely from elfread.c.  Modified
 >     	most functions.
 >     	* build-id.h: New file.
 >     	* dwarf2read.c (dwarf2_get_dwz_file): Update for change to
 >     	bfd_get_alt_debug_link_info.  Verify dwz file's build-id.
 >     	Search for dwz file using build-id.
 >     	* elfread.c (build_id_bfd_get, build_id_verify)
 >     	(build_id_to_debug_filename, find_separate_debug_file): Remove.
 >     
 >     	* gdb.dwarf2/dwzbuildid.exp: New file.
 >     	* lib/dwarf.exp (Dwarf::_section): Add "flags" and "type"
 >     	parameters.
 >     	(Dwarf::_defer_output): Change "section" parameter to
 >     	"section_spec"; update.
 >     	(Dwarf::gnu_debugaltlink, Dwarf::_note, Dwarf::build_id): New
 >     	procs.

So code was moved to a different file and edited in one step
(instead of moving it, and then editing it in a separate pass)?

[not that I mind per se, but I thought doing things that way
was more than just "nice to have"]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-10-10  0:17   ` [PATCH] fix PR symtab/15597 Doug Evans
@ 2013-10-10  2:29     ` Tom Tromey
  2013-10-10 19:04       ` Doug Evans
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2013-10-10  2:29 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

Doug> So code was moved to a different file and edited in one step
Doug> (instead of moving it, and then editing it in a separate pass)?

In this case I thought it couldn't reasonably be moved verbatim, since,
IIRC, prior to my changes it required ELF bits from BFD, not allowed in
generic code.

Doug> [not that I mind per se, but I thought doing things that way
Doug> was more than just "nice to have"]

In your case the two changes had nothing to do with one another.  I
think that is a reasonable standard.  If you disagree, I'm sure we can
all agree on some other standard.

Tom

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] fix PR symtab/15597
  2013-10-10  2:29     ` Tom Tromey
@ 2013-10-10 19:04       ` Doug Evans
  0 siblings, 0 replies; 14+ messages in thread
From: Doug Evans @ 2013-10-10 19:04 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On Wed, Oct 9, 2013 at 7:29 PM, Tom Tromey <tromey@redhat.com> wrote:
> Doug> So code was moved to a different file and edited in one step
> Doug> (instead of moving it, and then editing it in a separate pass)?
>
> In this case I thought it couldn't reasonably be moved verbatim, since,
> IIRC, prior to my changes it required ELF bits from BFD, not allowed in
> generic code.

Assuming I'm interpreting this correctly,
I think what's allowed can give way to the teeny window in which a
multi-part patch gets checked in.

> Doug> [not that I mind per se, but I thought doing things that way
> Doug> was more than just "nice to have"]
>
> In your case the two changes had nothing to do with one another.  I
> think that is a reasonable standard.  If you disagree, I'm sure we can
> all agree on some other standard.

My case?
I've long since dropped from cache what that might be referring to so
I had to go looking ...
Are you referring to this?
https://sourceware.org/ml/gdb-patches/2013-09/msg00884.html

I didn't move a function to a new file, the desire to simplify
archeology is what this (sub-)thread is about.
You could be referring to a different patch of course. :-)

No worries.  As I say I don't mind.  I raise the issue because I was
led to believe archeology simplification (for lack of a better phrase,
blech) is sufficiently important, so I wanted to clear it up.
Perhaps if the amount of code being moved was much larger, or the
changes were much greater, you would have split the patch into two as
a matter of course?

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-10-10 19:04 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-08 17:37 [PATCH] fix PR symtab/15597 Tom Tromey
2013-08-10  5:36 ` Alan Modra
2013-08-12  8:41   ` Tom Tromey
2013-08-12  9:29     ` Alan Modra
2013-08-14 11:25 ` nick clifton
2013-10-08 19:38 ` Tom Tromey
2013-10-08 22:10   ` Hans-Peter Nilsson
2013-10-09  1:27     ` Tom Tromey
2013-10-09 14:09       ` Tom Tromey
2013-10-09 14:54         ` Build regression on 32-bit hosts [Re: [PATCH] fix PR symtab/15597] Jan Kratochvil
2013-10-09 15:46           ` Tom Tromey
2013-10-10  0:17   ` [PATCH] fix PR symtab/15597 Doug Evans
2013-10-10  2:29     ` Tom Tromey
2013-10-10 19:04       ` Doug Evans

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).