[PATCH] Handle DWARF 5 separate debug sections

[PATCH] Handle DWARF 5 separate debug sections

[Tom Tromey]

DWARF 5 standardized the .gnu_debugaltlink section that dwz emits in
multi-file mode.  This is handled via some new forms, and a new
.debug_sup section.

This patch adds support for this to gdb.  It is largely
straightforward, I think, though one oddity is that I chose not to
have this code search the system build-id directories for the
supplementary file.  My feeling was that, while it makes sense for a
distro to unify the build-id concept with the hash stored in the
.debug_sup section, there's no intrinsic need to do so.

I also chose to put the .debug_sup parsing directly in gdb rather than
in BFD.  This is perhaps cheating a bit.

This patch requires my earlier 'dwz' series.

gdb/ChangeLog
2021-02-21  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (struct partial_die_info) <is_dwz, spec_is_dwz>:
	Update comments.
	(skip_one_die): Add DWARF 5 forms.
	(process_imported_unit_die, partial_die_info::read)
	(partial_die_info::read): Update.
	(read_attribute_value, dwarf2_const_value_attr): Add DWARF 5
	forms.
	(lookup_die_type): Update.
	(dump_die_shallow): Add DWARF 5 forms.
	(follow_die_ref): Update.
	(dwarf2_fetch_constant_bytes): Add DWARF 5 forms.
	* dwarf2/macro.c (skip_form_bytes): Add DWARF 5 form.
	* dwarf2/dwz.c (get_debug_sup_info, verify_id, read_alt_info): New
	functions.
	(dwz_search_other_debugdirs): Add 'dwarf5' parameter.
	(dwarf2_get_dwz_file): Update.
	* dwarf2/attribute.h (form_is_ref): Add DWARF 5 forms.
	(form_is_alt): New method.
	* dwarf2/attribute.c (attribute::form_is_string)
	(attribute::form_is_unsigned): Add DWARF 5 forms.

gdb/testsuite/ChangeLog
2021-02-21  Tom Tromey  <tom@tromey.com>

	* gdb.dwarf2/dwznolink.exp: Update expected output.
	* gdb.dwarf2/dwzbuildid.exp: Also test .debug_sup.
---
 gdb/ChangeLog                           |  23 +++
 gdb/dwarf2/attribute.c                  |   5 +-
 gdb/dwarf2/attribute.h                  |  14 +-
 gdb/dwarf2/dwz.c                        | 172 ++++++++++++++---
 gdb/dwarf2/macro.c                      |   1 +
 gdb/dwarf2/read.c                       |  30 ++-
 gdb/testsuite/ChangeLog                 |   5 +
 gdb/testsuite/gdb.dwarf2/dwzbuildid.exp | 245 +++++++++++++-----------
 gdb/testsuite/gdb.dwarf2/dwznolink.exp  |   2 +-
 gdb/testsuite/lib/dwarf.exp             |  18 ++
 10 files changed, 357 insertions(+), 158 deletions(-)

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 3205b0f7d29..7a73b20cc38 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -74,7 +74,8 @@ attribute::form_is_string () const
 	  || form == DW_FORM_strx3
 	  || form == DW_FORM_strx4
 	  || form == DW_FORM_GNU_str_index
-	  || form == DW_FORM_GNU_strp_alt);
+	  || form == DW_FORM_GNU_strp_alt
+	  || form == DW_FORM_strp_sup);
 }
 
 /* See attribute.h.  */
@@ -170,6 +171,8 @@ attribute::form_is_unsigned () const
 {
   return (form == DW_FORM_ref_addr
 	  || form == DW_FORM_GNU_ref_alt
+	  || form == DW_FORM_ref_sup4
+	  || form == DW_FORM_ref_sup8
 	  || form == DW_FORM_data2
 	  || form == DW_FORM_data4
 	  || form == DW_FORM_data8
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index 56776d64ed3..5490b670ffa 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -133,7 +133,9 @@ struct attribute
 	    || form == DW_FORM_ref4
 	    || form == DW_FORM_ref8
 	    || form == DW_FORM_ref_udata
-	    || form == DW_FORM_GNU_ref_alt);
+	    || form == DW_FORM_GNU_ref_alt
+	    || form == DW_FORM_ref_sup4
+	    || form == DW_FORM_ref_sup8);
   }
 
   /* Check if the attribute's form is a DW_FORM_block*
@@ -147,6 +149,16 @@ struct attribute
   /* Check if the attribute's form is an unsigned integer form.  */
   bool form_is_unsigned () const;
 
+  /* Check if attribute's form refers to the separate "dwz" file.
+     This is only useful for references to the .debug_info section,
+     not to the supplementary .debug_str section.  */
+  bool form_is_alt () const
+  {
+    return (form == DW_FORM_GNU_ref_alt
+	    || form == DW_FORM_ref_sup4
+	    || form == DW_FORM_ref_sup8);
+  }
+
   /* Check if the attribute's form is a form that requires
      "reprocessing".  */
   bool form_requires_reprocessing () const;
diff --git a/gdb/dwarf2/dwz.c b/gdb/dwarf2/dwz.c
index f9d5db6b48a..3cde6ff3477 100644
--- a/gdb/dwarf2/dwz.c
+++ b/gdb/dwarf2/dwz.c
@@ -22,6 +22,7 @@
 
 #include "build-id.h"
 #include "debuginfod-support.h"
+#include "dwarf2/leb.h"
 #include "dwarf2/read.h"
 #include "dwarf2/sect-names.h"
 #include "filenames.h"
@@ -93,6 +94,125 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
     }
 }
 
+/* Like bfd_get_alt_debug_link_info, but looks at the .debug_sup
+   section.  Returns true on success and fills in the out parameters;
+   false on failure.  */
+
+static bool
+get_debug_sup_info (bfd *abfd,
+		    gdb::unique_xmalloc_ptr<char> *filename,
+		    size_t *buildid_len,
+		    gdb::unique_xmalloc_ptr<bfd_byte> *buildid)
+{
+  asection *sect = bfd_get_section_by_name (abfd, ".debug_sup");
+  if (sect == nullptr)
+    return false;
+
+  bfd_byte *contents;
+  if (!bfd_malloc_and_get_section (abfd, sect, &contents))
+    return false;
+
+  gdb::unique_xmalloc_ptr<bfd_byte> content_holder (contents);
+  bfd_size_type size = bfd_section_size (sect);
+
+  /* Version of this section.  */
+  if (size < 4)
+    return false;
+  unsigned int version = read_2_bytes (abfd, contents);
+  contents += 2;
+  size -= 2;
+  if (version != 5)
+    return false;
+
+  /* Skip the is_supplementary value.  We already ensured there were
+     enough bytes, above.  */
+  ++contents;
+  --size;
+
+  /* The spec says that in the supplementary file, this must be \0,
+     but it doesn't seem very important.  */
+  *filename = make_unique_xstrdup ((const char *) contents);
+  int len = strlen (filename->get ());
+  contents += len + 1;
+  size -= len + 1;
+
+  if (size == 0)
+    return false;
+
+  unsigned int bytes_read;
+  *buildid_len = read_unsigned_leb128 (abfd, contents, &bytes_read);
+  contents += bytes_read;
+  size -= bytes_read;
+
+  if (size < *buildid_len)
+    return false;
+
+  if (*buildid_len != 0)
+    buildid->reset ((bfd_byte *) xmemdup (contents, *buildid_len,
+					  *buildid_len));
+
+  return true;
+}
+
+/* Validate that ABFD matches the given BUILDID.  If DWARF5 is true,
+   then this is done by examining the .debug_sup data.  */
+
+static bool
+verify_id (bfd *abfd, size_t len, const bfd_byte *buildid, bool dwarf5)
+{
+  if (!bfd_check_format (abfd, bfd_object))
+    return false;
+
+  if (dwarf5)
+    {
+      size_t new_len;
+      gdb::unique_xmalloc_ptr<bfd_byte> new_id;
+      gdb::unique_xmalloc_ptr<char> ignore;
+
+      if (!get_debug_sup_info (abfd, &ignore, &new_len, &new_id))
+	return false;
+      return (len == new_len
+	      && memcmp (buildid, new_id.get (), len) == 0);
+    }
+  else
+    return build_id_verify (abfd, len, buildid);
+}
+
+/* Find either the .debug_sup or .gnu_debugaltlink section and return
+   its contents.  Returns true on success and sets out parameters, or
+   false if nothing is found.  */
+
+static bool
+read_alt_info (bfd *abfd, gdb::unique_xmalloc_ptr<char> *filename,
+	       size_t *buildid_len,
+	       gdb::unique_xmalloc_ptr<bfd_byte> *buildid,
+	       bool *dwarf5)
+{
+  if (get_debug_sup_info (abfd, filename, buildid_len, buildid))
+    {
+      *dwarf5 = true;
+      return true;
+    }
+
+  bfd_size_type buildid_len_arg;
+  bfd_set_error (bfd_error_no_error);
+  bfd_byte *buildid_out;
+  filename->reset (bfd_get_alt_debug_link_info (abfd, &buildid_len_arg,
+						&buildid_out));
+  if (*filename == nullptr)
+    {
+      if (bfd_get_error () == bfd_error_no_error)
+	return false;
+      error (_("could not read '.gnu_debugaltlink' section: %s"),
+	     bfd_errmsg (bfd_get_error ()));
+    }
+
+  *buildid_len = buildid_len_arg;
+  buildid->reset (buildid_out);
+  *dwarf5 = false;
+  return true;
+}
+
 /* Attempt to find a .dwz file (whose full path is represented by
    FILENAME) in all of the specified debug file directories provided.
 
@@ -101,7 +221,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, dwz_file *dwz_file)
 
 static gdb_bfd_ref_ptr
 dwz_search_other_debugdirs (std::string &filename, bfd_byte *buildid,
-			    size_t buildid_len)
+			    size_t buildid_len, bool dwarf5)
 {
   /* Let's assume that the path represented by FILENAME has the
      "/.dwz/" subpath in it.  This is what (most) GNU/Linux
@@ -169,7 +289,7 @@ dwz_search_other_debugdirs (std::string &filename, bfd_byte *buildid,
       if (dwz_bfd == nullptr)
 	continue;
 
-      if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
+      if (!verify_id (dwz_bfd.get (), buildid_len, buildid, dwarf5))
 	{
 	  dwz_bfd.reset (nullptr);
 	  continue;
@@ -187,33 +307,20 @@ dwz_search_other_debugdirs (std::string &filename, bfd_byte *buildid,
 struct dwz_file *
 dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
 {
-  bfd_size_type buildid_len_arg;
-  size_t buildid_len;
-  bfd_byte *buildid;
-
   if (per_bfd->dwz_file != NULL)
     return per_bfd->dwz_file.get ();
 
-  bfd_set_error (bfd_error_no_error);
-  gdb::unique_xmalloc_ptr<char> data
-    (bfd_get_alt_debug_link_info (per_bfd->obfd,
-				  &buildid_len_arg, &buildid));
-  if (data == NULL)
+  size_t buildid_len;
+  gdb::unique_xmalloc_ptr<bfd_byte> buildid;
+  gdb::unique_xmalloc_ptr<char> data;
+  bool dwarf5;
+  if (!read_alt_info (per_bfd->obfd, &data, &buildid_len, &buildid, &dwarf5))
     {
-      if (bfd_get_error () == bfd_error_no_error)
-	{
-	  if (!require)
-	    return nullptr;
-	  error (_("could not read '.gnu_debugaltlink' section"));
-	}
-      error (_("could not read '.gnu_debugaltlink' section: %s"),
-	     bfd_errmsg (bfd_get_error ()));
+      if (!require)
+	return nullptr;
+      error (_("could not read '.debug_sup' or '.gnu_debugaltlink' section"));
     }
 
-  gdb::unique_xmalloc_ptr<bfd_byte> buildid_holder (buildid);
-
-  buildid_len = (size_t) buildid_len_arg;
-
   std::string filename = data.get ();
 
   if (!IS_ABSOLUTE_PATH (filename.c_str ()))
@@ -229,18 +336,22 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
   gdb_bfd_ref_ptr dwz_bfd (gdb_bfd_open (filename.c_str (), gnutarget));
   if (dwz_bfd != NULL)
     {
-      if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
+      if (!verify_id (dwz_bfd.get (), buildid_len, buildid.get (), dwarf5))
 	dwz_bfd.reset (nullptr);
     }
 
-  if (dwz_bfd == NULL)
-    dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid);
+  /* In DWARF 5, the filename in the main debug file is either
+     absolute, or relative to the debug file -- so we don't search the
+     system directories.  */
+  if (dwz_bfd == NULL && !dwarf5)
+    dwz_bfd = build_id_to_debug_bfd (buildid_len, buildid.get ());
 
   if (dwz_bfd == nullptr)
     {
       /* If the user has provided us with different
 	 debug file directories, we can try them in order.  */
-      dwz_bfd = dwz_search_other_debugdirs (filename, buildid, buildid_len);
+      dwz_bfd = dwz_search_other_debugdirs (filename, buildid.get (),
+					    buildid_len, dwarf5);
     }
 
   if (dwz_bfd == nullptr)
@@ -248,7 +359,7 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
       gdb::unique_xmalloc_ptr<char> alt_filename;
       const char *origname = bfd_get_filename (per_bfd->obfd);
 
-      scoped_fd fd (debuginfod_debuginfo_query (buildid,
+      scoped_fd fd (debuginfod_debuginfo_query (buildid.get (),
 						buildid_len,
 						origname,
 						&alt_filename));
@@ -261,13 +372,14 @@ dwarf2_get_dwz_file (dwarf2_per_bfd *per_bfd, bool require)
 	  if (dwz_bfd == nullptr)
 	    warning (_("File \"%s\" from debuginfod cannot be opened as bfd"),
 		     alt_filename.get ());
-	  else if (!build_id_verify (dwz_bfd.get (), buildid_len, buildid))
+	  else if (!verify_id (dwz_bfd.get (), buildid_len, buildid.get (),
+			       dwarf5))
 	    dwz_bfd.reset (nullptr);
 	}
     }
 
   if (dwz_bfd == NULL)
-    error (_("could not find '.gnu_debugaltlink' file for %s"),
+    error (_("could not find supplementary DWARF file for %s"),
 	   bfd_get_filename (per_bfd->obfd));
 
   std::unique_ptr<struct dwz_file> result
diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
index 2ecebe6173c..b23ae611963 100644
--- a/gdb/dwarf2/macro.c
+++ b/gdb/dwarf2/macro.c
@@ -266,6 +266,7 @@ skip_form_bytes (bfd *abfd, const gdb_byte *bytes, const gdb_byte *buffer_end,
     case DW_FORM_sec_offset:
     case DW_FORM_strp:
     case DW_FORM_GNU_strp_alt:
+    case DW_FORM_strp_sup:
       bytes += offset_size;
       break;
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 31f34dc6047..32212401bde 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1080,10 +1080,12 @@ struct partial_die_info : public allocate_on_obstack
     /* Flag set if fixup has been called on this die.  */
     unsigned int fixup_called : 1;
 
-    /* Flag set if DW_TAG_imported_unit uses DW_FORM_GNU_ref_alt.  */
+    /* Flag set if DW_TAG_imported_unit uses DW_FORM_GNU_ref_alt,
+       DW_FORM_ref_sup4, or DW_FORM_ref_sup8.  */
     unsigned int is_dwz : 1;
 
-    /* Flag set if spec_offset uses DW_FORM_GNU_ref_alt.  */
+    /* Flag set if spec_offset uses DW_FORM_GNU_ref_alt,
+       DW_FORM_ref_sup4, or DW_FORM_ref_sup8.  */
     unsigned int spec_is_dwz : 1;
 
     unsigned int canonical_name : 1;
@@ -8811,11 +8813,13 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	case DW_FORM_data4:
 	case DW_FORM_ref4:
 	case DW_FORM_strx4:
+	case DW_FORM_ref_sup4:
 	  info_ptr += 4;
 	  break;
 	case DW_FORM_data8:
 	case DW_FORM_ref8:
 	case DW_FORM_ref_sig8:
+	case DW_FORM_ref_sup8:
 	  info_ptr += 8;
 	  break;
 	case DW_FORM_data16:
@@ -8828,6 +8832,7 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	case DW_FORM_sec_offset:
 	case DW_FORM_strp:
 	case DW_FORM_GNU_strp_alt:
+	case DW_FORM_strp_sup:
 	  info_ptr += cu->header.offset_size;
 	  break;
 	case DW_FORM_exprloc:
@@ -9985,7 +9990,7 @@ process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu)
   if (attr != NULL)
     {
       sect_offset sect_off = attr->get_ref_die_offset ();
-      bool is_dwz = (attr->form == DW_FORM_GNU_ref_alt || cu->per_cu->is_dwz);
+      bool is_dwz = attr->form_is_alt () || cu->per_cu->is_dwz;
       dwarf2_per_objfile *per_objfile = cu->per_objfile;
       dwarf2_per_cu_data *per_cu
 	= dwarf2_find_containing_comp_unit (sect_off, is_dwz, per_objfile);
@@ -19541,8 +19546,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	case DW_AT_extension:
 	  has_specification = 1;
 	  spec_offset = attr.get_ref_die_offset ();
-	  spec_is_dwz = (attr.form == DW_FORM_GNU_ref_alt
-				   || cu->per_cu->is_dwz);
+	  spec_is_dwz = attr.form_is_alt () || cu->per_cu->is_dwz;
 	  break;
 	case DW_AT_sibling:
 	  /* Ignore absolute siblings, they might point outside of
@@ -19601,8 +19605,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	  if (tag == DW_TAG_imported_unit)
 	    {
 	      d.sect_off = attr.get_ref_die_offset ();
-	      is_dwz = (attr.form == DW_FORM_GNU_ref_alt
-				  || cu->per_cu->is_dwz);
+	      is_dwz = attr.form_is_alt () || cu->per_cu->is_dwz;
 	    }
 	  break;
 
@@ -20233,10 +20236,12 @@ read_attribute_value (const struct die_reader_specs *reader,
       info_ptr += 2;
       break;
     case DW_FORM_data4:
+    case DW_FORM_ref_sup4:
       attr->set_unsigned (read_4_bytes (abfd, info_ptr));
       info_ptr += 4;
       break;
     case DW_FORM_data8:
+    case DW_FORM_ref_sup8:
       attr->set_unsigned (read_8_bytes (abfd, info_ptr));
       info_ptr += 8;
       break;
@@ -20286,6 +20291,7 @@ read_attribute_value (const struct die_reader_specs *reader,
 	}
       /* FALLTHROUGH */
     case DW_FORM_GNU_strp_alt:
+    case DW_FORM_strp_sup:
       {
 	dwz_file *dwz = dwarf2_get_dwz_file (per_objfile->per_bfd, true);
 	LONGEST str_offset = cu_header->read_offset (abfd, info_ptr,
@@ -22381,6 +22387,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
     case DW_FORM_strx:
     case DW_FORM_GNU_str_index:
     case DW_FORM_GNU_strp_alt:
+    case DW_FORM_strp_sup:
       /* The string is already allocated on the objfile obstack, point
 	 directly to it.  */
       *bytes = (const gdb_byte *) attr->as_string ();
@@ -22584,7 +22591,7 @@ lookup_die_type (struct die_info *die, const struct attribute *attr,
 
   /* First see if we have it cached.  */
 
-  if (attr->form == DW_FORM_GNU_ref_alt)
+  if (attr->form_is_alt ())
     {
       struct dwarf2_per_cu_data *per_cu;
       sect_offset sect_off = attr->get_ref_die_offset ();
@@ -23265,6 +23272,8 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	  fputs_filtered (hex_string (die->attrs[i].as_unsigned ()), f);
 	  break;
 	case DW_FORM_GNU_ref_alt:
+	case DW_FORM_ref_sup4:
+	case DW_FORM_ref_sup8:
 	  fprintf_unfiltered (f, "alt ref address: ");
 	  fputs_filtered (hex_string (die->attrs[i].as_unsigned ()), f);
 	  break;
@@ -23298,6 +23307,7 @@ dump_die_shallow (struct ui_file *f, int indent, struct die_info *die)
 	case DW_FORM_strx:
 	case DW_FORM_GNU_str_index:
 	case DW_FORM_GNU_strp_alt:
+	case DW_FORM_strp_sup:
 	  fprintf_unfiltered (f, "string: \"%s\" (%s canonicalized)",
 			      die->attrs[i].as_string ()
 			      ? die->attrs[i].as_string () : "",
@@ -23500,8 +23510,7 @@ follow_die_ref (struct die_info *src_die, const struct attribute *attr,
   struct die_info *die;
 
   die = follow_die_offset (sect_off,
-			   (attr->form == DW_FORM_GNU_ref_alt
-			    || cu->per_cu->is_dwz),
+			   attr->form_is_alt () || cu->per_cu->is_dwz,
 			   ref_cu);
   if (!die)
     error (_("Dwarf Error: Cannot find DIE at %s referenced from DIE "
@@ -23710,6 +23719,7 @@ dwarf2_fetch_constant_bytes (sect_offset sect_off,
     case DW_FORM_strx:
     case DW_FORM_GNU_str_index:
     case DW_FORM_GNU_strp_alt:
+    case DW_FORM_strp_sup:
       /* The string is already allocated on the objfile obstack, point
 	 directly to it.  */
       {
diff --git a/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp b/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
index 3b6315642fe..c78c25011f7 100644
--- a/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
+++ b/gdb/testsuite/gdb.dwarf2/dwzbuildid.exp
@@ -25,146 +25,161 @@ if {[is_remote host]} {
     return 0
 }
 
+foreach_with_prefix scenario {gnu dwarf5} {
+    # 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
+	    global scenario
+
+	    if {$scenario == "gnu"} {
+		gnu_debugaltlink $dwzname $buildid
+	    } else {
+		debug_sup 0 $dwzname $buildid
+	    }
 
-# 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}} {
+	    # 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]
+    # 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 int_label int_label2
+	Dwarf::assemble $asm_file {
+	    declare_labels int_label int_label2
 
-	upvar buildid buildid
-	upvar value value
+	    global scenario
+	    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}
-		}
+	    if {$scenario == "gnu"} {
+		build_id $buildid
+	    } else {
+		debug_sup 1 "" $buildid
+	    }
 
-		constant {
-		    {name the_int}
-		    {type :$int_label2}
-		    {const_value $value data1}
+	    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 debugdir [standard_output_file {}]
-set basedir $debugdir/.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] != ""} {
+    if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.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
+    # 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 debugdir [standard_output_file {}]
+    set basedir $debugdir/.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
+	}
+    }
 
-# Link the executables.
-if {[gdb_compile [list ${binfile}1.o ${binfile}2.o] ${binfile}-ok \
-	 executable {}] != ""} {
-    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
 
-if {[gdb_compile [list ${binfile}1.o ${binfile}4.o] ${binfile}-mismatch \
-	 executable {quiet}] != ""} {
-    return -1
-}
+    # 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}6.o] ${binfile}-fallback \
-	 executable {}] != ""} {
-    return -1
-}
+    if {[gdb_compile [list ${binfile}1.o ${binfile}4.o] ${binfile}-mismatch \
+	     executable {quiet}] != ""} {
+	return -1
+    }
 
+    if {[gdb_compile [list ${binfile}1.o ${binfile}6.o] ${binfile}-fallback \
+	     executable {}] != ""} {
+	return -1
+    }
+
+    set tests {ok mismatch}
+    if {$scenario == "gnu"} {
+	lappend tests fallback
+    }
 
-foreach testname {ok mismatch fallback} {
-    with_test_prefix $testname {
-	gdb_exit
-	gdb_start
-	gdb_reinitialize_dir $srcdir/$subdir
+    foreach testname $tests {
+	with_test_prefix $testname {
+	    gdb_exit
+	    gdb_start
+	    gdb_reinitialize_dir $srcdir/$subdir
 
-	gdb_test_no_output "set debug-file-directory $debugdir" \
-	    "set debug-file-directory"
+	    gdb_test_no_output "set debug-file-directory $debugdir" \
+		"set debug-file-directory"
 
-	gdb_load ${binfile}-${testname}
+	    gdb_load ${binfile}-${testname}
 
-	if {[runto_main]} {
-	    if {$testname == "mismatch"} {
-		gdb_test "print the_int" \
-		    "(No symbol table is loaded|No symbol \"the_int\" in current context).*"
-	    } else {
-		gdb_test "print the_int" " = 99"
+	    if {[runto_main]} {
+		if {$testname == "mismatch"} {
+		    gdb_test "print the_int" \
+			"(No symbol table is loaded|No symbol \"the_int\" in current context).*"
+		} else {
+		    gdb_test "print the_int" " = 99"
+		}
 	    }
 	}
     }
diff --git a/gdb/testsuite/gdb.dwarf2/dwznolink.exp b/gdb/testsuite/gdb.dwarf2/dwznolink.exp
index 98976d7e59c..8c3a0106caa 100644
--- a/gdb/testsuite/gdb.dwarf2/dwznolink.exp
+++ b/gdb/testsuite/gdb.dwarf2/dwznolink.exp
@@ -56,5 +56,5 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 
 gdb_test "file -readnow $binfile" \
-    "could not read '.gnu_debugaltlink' section" \
+    "could not read '.debug_sup' or '.gnu_debugaltlink' section" \
     "file $testfile"
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index f8fbd381810..3b310abd952 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -1992,6 +1992,24 @@ namespace eval Dwarf {
 	}
     }
 
+    # Emit a .debug_sup section with the given file name and build-id.
+    # The buildid should be represented as a hexadecimal string, like
+    # "ffeeddcc".
+    proc debug_sup {is_sup filename buildid} {
+	_defer_output .debug_sup {
+	    # The version.
+	    _op .2byte 0x5
+	    # Supplementary marker.
+	    _op .byte $is_sup
+	    _op .ascii [_quote $filename]
+	    set len [expr {[string length $buildid] / 2}]
+	    _op .uleb128 $len
+	    foreach {a b} [split $buildid {}] {
+		_op .byte 0x$a$b
+	    }
+	}
+    }
+
     proc _note {type name hexdata} {
 	set namelen [expr [string length $name] + 1]
 
-- 
2.26.2

build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Mark Wielaard]

Hi,

I am adding the elfutils and dwz mailinglists to CC because I think
you stumbled upon a silent assumption debuginfod makes which would
be good to make explicit (or maybe change?)

Context is that dwz 0.15 (still not released yet) can now produce
standardized DWARF Supplementary Files using a .debug_sup section
when using --dwarf-5 -m multifile. See this bug report:
https://sourceware.org/bugzilla/show_bug.cgi?id=27440

The full patch to support that in gdb is here:
https://sourceware.org/pipermail/gdb-patches/2021-February/176508.html

But what I like to discuss is this part of Tom's email:

On Sun, Feb 21, 2021 at 04:18:10PM -0700, Tom Tromey wrote:
> DWARF 5 standardized the .gnu_debugaltlink section that dwz emits in
> multi-file mode.  This is handled via some new forms, and a new
> .debug_sup section.
> 
> This patch adds support for this to gdb.  It is largely
> straightforward, I think, though one oddity is that I chose not to
> have this code search the system build-id directories for the
> supplementary file.  My feeling was that, while it makes sense for a
> distro to unify the build-id concept with the hash stored in the
> .debug_sup section, there's no intrinsic need to do so.

Tom is correct. Technically a supplemental (alt) file id isn't a
build-id. But it does smell like one. It is a large globally unique
identifier that can be expressed as hex string. And the GNU extension
defining alt files for DWARF < 5 (and still with DWARF5 currently
by default because no consumer yet supports .debug_sup) will put
it in a .note.gnu.build-id NOTE section as GNU_BUILD_ID.

This has the nice side-effect that a distro will most likely make
it available as /usr/lib/debug/.build-id/xx/yyyy.debug file. And
that "build-id" is how you request it from debuginfod (you request
/buildid/BUILDID/debuginfo).

Now technically that is cheating and confusing two concepts,
the build-id and supplemental file id. But I was personally assuming
we would extend it to also to other things like dwo IDs (which are
again almost identical globally unique identifiers for files).
There one question would be if you would get the .dwo file or the
.dwp file in which is was embedded (I would vote for the second).

One consequence of conflating all these IDs is that it isn't immediately
clear what a debuginfod request for /buildid/BUILDID/executable should
return (probably nothing). Or if /buildid/BUILDID/source/SOURCE/FILE
requests also (should) work for other IDs than build-ids.

Any opinions on whether we should split these concepts out and introduce
separate request mechanisms per ID-kind, or simply assume a globally
unique id is globally unique and we just clarify what it means to use
a build-id, sup_checksum or dwo_id together with a request for an
executable, debuginfo or source/file?

Thanks,

Mark

Re: build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Nick Clifton]

Hi Mark,

> Context is that dwz 0.15 (still not released yet) can now produce
> standardized DWARF Supplementary Files using a .debug_sup section
> when using --dwarf-5 -m multifile. See this bug report:
> https://sourceware.org/bugzilla/show_bug.cgi?id=27440

Is there somewhere that I can lay my hands on a file containing a
.debug_sup section and its corresponding supplimentary file ?  I
would like to test the binutils to make sure that they can support
them too.

Cheers
   Nick

Re: build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Mark Wielaard]

Hi Nick,

On Wed, Feb 24, 2021 at 05:00:14PM +0000, Nick Clifton wrote:
> > Context is that dwz 0.15 (still not released yet) can now produce
> > standardized DWARF Supplementary Files using a .debug_sup section
> > when using --dwarf-5 -m multifile. See this bug report:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=27440
> 
> Is there somewhere that I can lay my hands on a file containing a
> .debug_sup section and its corresponding supplimentary file ?  I
> would like to test the binutils to make sure that they can support
> them too.

Currently you need dwz git trunk (hopefully we will do a real release
by Monday?). The following will get you a .sup file for dwz itself:

$ git clone git://sourceware.org/git/dwz.git
$ cd dwz
$ ./configure
$ make
$ cp dwz one
$ cp dwz two
$ dwz --dwarf-5 -m sup one two

If you already process .gnu_debugaltlink then processing the
.debug_sup shouldn't be too hard. Just don't expect there to be a
.note.gnu.build-id. Also note that instead of DW_FORM_GNU_alt_ref and
DW_FORM_GNU_alt_strp the one and two files will contain
DW_FORM_ref_sup and DW_FORM_ref_strp.

The formal description of the .debug_sup section can be found in
section 7.3.6 DWARF Supplementary Object Files
http://dwarfstd.org/doc/DWARF5.pdf

Cheers,

Mark

Re: build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Nick Clifton]

Hi Mark,

> $ git clone git://sourceware.org/git/dwz.git
> $ cd dwz
> $ ./configure
> $ make
> $ cp dwz one
> $ cp dwz two
> $ dwz --dwarf-5 -m sup one two

Thanks.  Using those files as a guide I have added some initial support for displaying and following .debug_sup sections to readelf and objdump.

Cheers
   Nick

Re: build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Matt Schulte]

Hi Mark,

My apologies for bringing this up so late. I was just re-reading this thread
while looking at how to find a .dwp for a given binary.

> But I was personally assuming we would extend it to also to other things like
> dwo IDs (which are again almost identical globally unique identifiers for
> files)

I'm concerned about using dwo IDs to index debuginfod. They are only 64-bits and
there will be many more dwo IDs than build ids or supplemental file ids since
there is 1 per compile unit.  Assuming dwo IDs are randomly distributed, once we
have ~600,000,000 dwo IDs we have a 1% chance of a collision. ~600,000,000 =
sqrt(2 * 2^64 * 0.01) (I think I did that math right but forgive me if not).

Maybe that's an ok number? (I tried to estimate the number of compile units in
one distro's release, but do not have a good way of doing that quickly)

What about using `/buildid/BUILDID/dwp` instead? This is not a perfect solution,
since (currently) no one puts the build-id into the *.dwp file, but it does get
around this collision problem.

-Matt

Re: build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Frank Ch. Eigler]

Hi -

> I'm concerned about using dwo IDs to index debuginfod. They are only
> 64-bits and there will be many more dwo IDs than build ids or
> supplemental file ids [...]

AIUI, -gsplit-dwarf is more suitable for development/scratch builds
than for distro binaries.  If distros agree, then I would not expect
.dwo files to show up in distro-wide debuginfod services, but rather
within developers' own build trees.  Then debuginfod indexing
collisions would only be a risk within a particular local set of trees
(if serviced by a local debuginfod), rather than distro wide or wider.

> What about using `/buildid/BUILDID/dwp` instead? This is not a
> perfect solution, since (currently) no one puts the build-id into
> the *.dwp file, but it does get around this collision problem.

The hypothetical problem is collision between dwo/dwp files, not
between dwo/dwp and normal buildid dwarf files, right?  In that case,
are you talking about two levels of indexing (buildid of final linked
binary + dwo_id)?  That would resemble the indexing work required from
debuginfod to match up binaries with their source files plus binaries
with dwz supplemental files).

- FChE

Re: build-ids, .debug_sup and other IDs (Was: [PATCH] Handle DWARF 5 separate debug sections)

[Matt Schulte]

Thanks for the thoughts!

> AIUI, -gsplit-dwarf is more suitable for development/scratch builds
> than for distro binaries.  If distros agree, then I would not expect
> .dwo files to show up in distro-wide debuginfod services, but rather
> within developers' own build trees.

That's a good point. My concerns are only valid if distros decide to
start building packages using -gsplit-dwarf and dwp to package up
the .dwo files into one .dwp file.

I also agree that split dwarfs (split dwarves?) are more suitable for
local builds than for distro builds. The one advantage I can think
of that split dwarfs offer distro binaries is a faster build for
larger packages (since dwp does not do all the relocations the
linker would normally do). But I don't know enough about building
packages to say what will happen in the future.

> The hypothetical problem is collision between dwo/dwp files, not
> between dwo/dwp and normal buildid dwarf files, right?

That's correct.

> In that case, are you talking about two levels of indexing (buildid
> of final linked binary + dwo_id)?

I was suggesting one level of indexing. The buildid of the final linked
binary would be used to reference the dwp file directly. This solution
would not work for individual dwo files. For individual dwo files we
could still use the dwo_id as they should only be for local builds.

-Matt

Re: build-ids, .debug_sup and other IDs

[Tom Tromey]

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

>> This patch adds support for this to gdb.  It is largely
>> straightforward, I think, though one oddity is that I chose not to
>> have this code search the system build-id directories for the
>> supplementary file.  My feeling was that, while it makes sense for a
>> distro to unify the build-id concept with the hash stored in the
>> .debug_sup section, there's no intrinsic need to do so.

Mark> Any opinions on whether we should split these concepts out and introduce
Mark> separate request mechanisms per ID-kind, or simply assume a globally
Mark> unique id is globally unique and we just clarify what it means to use
Mark> a build-id, sup_checksum or dwo_id together with a request for an
Mark> executable, debuginfo or source/file?

FWIW I looked a little at unifying these.  For example,
bfdopncls.c:bfd_get_alt_debug_link_info could look at both the build-id
and .debug_sup.

But, this seemed a bit weird.  What if both appear and they are
different?  Then a single API isn't so great -- you want to check the ID
corresponding to whatever was in the original file.

Probably I should have stuck some of the new code into BFD though.
It's not too late to do that at least.

I suppose a distro can ensure that the IDs are always equal.  Maybe
debuginfod could also just require this.

Tom

Re: build-ids, .debug_sup and other IDs

[Frank Ch. Eigler]

Hi -

> FWIW I looked a little at unifying these.  For example,
> bfdopncls.c:bfd_get_alt_debug_link_info could look at both the build-id
> and .debug_sup.
> 
> But, this seemed a bit weird.  What if both appear and they are
> different?  Then a single API isn't so great -- you want to check the ID
> corresponding to whatever was in the original file.

If both appear and are different, can we characterize the elf file as
malformed?  Does our current tooling produce such files?  If it's an
abnormality (requires special elf manipulation or whatever), we could
avoid bending backward for this case, and tune the tools to the
Normal.

> [...]
> I suppose a distro can ensure that the IDs are always equal.

Via debugedit for example?

> Maybe debuginfod could also just require this.

Or debuginfod could export the content under -both- IDs, if there were
two valid candidates, and just go with the flow.  Let the clients
choose which ID they prefer to look up by.


- FChE

Re: build-ids, .debug_sup and other IDs

[Jakub Jelinek]

On Thu, Feb 25, 2021 at 11:42:45AM -0500, Frank Ch. Eigler via Dwz wrote:
> > FWIW I looked a little at unifying these.  For example,
> > bfdopncls.c:bfd_get_alt_debug_link_info could look at both the build-id
> > and .debug_sup.
> > 
> > But, this seemed a bit weird.  What if both appear and they are
> > different?  Then a single API isn't so great -- you want to check the ID
> > corresponding to whatever was in the original file.
> 
> If both appear and are different, can we characterize the elf file as
> malformed?

Unsure, the DWARF spec only talks about .debug_sup, the NOTE is a GNU
extension.

> Does our current tooling produce such files?  If it's an

dwz without --dwarf-5 produces .gnu_debugaltlink in the referrers and
.note.gnu.build-id in the supplemental object file.
For dwz --dwarf-5, if it produced a .note.gnu.build-id, it would produce
the same one, but I thought that if I produced that, then consumers could
keep using that instead of .debug_sup which is the only thing defined
in the standard, so in the end dwz --dwarf-5 only produces .debug_sup
on both the referrers side and on the side of supplemental object file
as DWARF specifies.

	Jakub

Re: build-ids, .debug_sup and other IDs

[Frank Ch. Eigler]

Hi -

> For dwz --dwarf-5, if it produced a .note.gnu.build-id, it would produce
> the same one, but I thought that if I produced that, then consumers could
> keep using that instead of .debug_sup which is the only thing defined
> in the standard, so in the end dwz --dwarf-5 only produces .debug_sup
> on both the referrers side and on the side of supplemental object file
> as DWARF specifies.

Right, but build-ids are still in normal binaries -- just not the
dwz-commonized files created by "dwz --dwarf-5"?  So our toolchains
still process build-ids routinely for all the other uses.  By omitting
the build-id on the dwz-generated files, we're forcing a flag day on
all our consumer tools.  (Does dwz'd dwarf5 even work on gdb
etc. now?)  ISTM tool backward compatibility is more important, so
I would suggest dwz generate -both- identifiers.

- FChE

Re: build-ids, .debug_sup and other IDs

[Tom Tromey]

Frank> (Does dwz'd dwarf5 even work on gdb
Frank> etc. now?)

It doesn't, this thread started because I sent a patch to change gdb to
read .debug_sup.  This hasn't landed yet.

Tom

Re: build-ids, .debug_sup and other IDs

[Tom Tromey]

>> But, this seemed a bit weird.  What if both appear and they are
>> different?  Then a single API isn't so great -- you want to check the ID
>> corresponding to whatever was in the original file.

Frank> If both appear and are different, can we characterize the elf file as
Frank> malformed?

Not really, nothing specifies that these must be the same.

Frank> Or debuginfod could export the content under -both- IDs, if there were
Frank> two valid candidates, and just go with the flow.  Let the clients
Frank> choose which ID they prefer to look up by.

There's a namespace problem here.  You could, in theory, have executable
A with build id AAAA, and also executable B with debug_sup id also AAAA.
This could be fixed with some kind of query parameter.  It would be easy
on the gdb side to supply this information.

Tom