[PATCH 0/8] Extension for looking up debug info by build-id

[PATCH 0/8] Extension for looking up debug info by build-id

[Andrew Burgess]

Patches #1 and #2 are bug fixes.

Patch #3 is a minor improvement.

Patch #4 is a conversion to the new(ish) debug scheme.

Patches #5, #6, & #7 add target_ops::fileio_stat() function.

Patch #8 adds an extension to how GDB looks for files based on
    build-id.

---

Andrew Burgess (8):
  gdb/fileio: fix errno for packets where an attachment is expected
  gdb: avoid duplicate search in build_id_to_bfd_suffix
  gdb: warn of slow remote file reading only after a successful open
  gdb: convert separate-debug-file to new(ish) debug scheme
  gdb: add target_fileio_stat, but no implementations yet
  gdb: add GDB side target_ops::fileio_stat implementation
  gdbserver: add gdbserver support for vFile::stat packet
  gdb: check for multiple matching build-id files

 gdb/NEWS                                      |   5 +
 gdb/build-id.c                                | 205 +++++++++++++-----
 gdb/doc/gdb.texinfo                           |  11 +
 gdb/inf-child.c                               |  15 ++
 gdb/inf-child.h                               |   2 +
 gdb/remote.c                                  | 122 ++++++++---
 gdb/symfile.c                                 |  45 ++--
 gdb/symfile.h                                 |  19 ++
 gdb/target.c                                  |  31 +++
 gdb/target.h                                  |  16 ++
 gdb/testsuite/gdb.base/build-id-seqno.c       |  22 ++
 gdb/testsuite/gdb.base/build-id-seqno.exp     | 133 ++++++++++++
 gdb/testsuite/gdb.server/build-id-seqno.c     |  22 ++
 gdb/testsuite/gdb.server/build-id-seqno.exp   | 198 +++++++++++++++++
 gdb/testsuite/gdb.server/remote-read-msgs.c   |  22 ++
 gdb/testsuite/gdb.server/remote-read-msgs.exp | 119 ++++++++++
 gdbserver/hostio.cc                           |  38 ++++
 17 files changed, 912 insertions(+), 113 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.exp
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.exp
 create mode 100644 gdb/testsuite/gdb.server/remote-read-msgs.c
 create mode 100644 gdb/testsuite/gdb.server/remote-read-msgs.exp


base-commit: 2db414c36b4f030782c2c8a24c916c3033261af0
-- 
2.25.4

[PATCH 1/8] gdb/fileio: fix errno for packets where an attachment is expected

[Andrew Burgess]

In remote.c lives remote_target::remote_hostio_send_command(), which
is used to handle sending a fileio packet to the remote, and for
parsing the reply packet.

Some commands, e.g. open, pwrite, close, send some arguments to the
remote, and then get back a single integer return value.

Other commands though, e.g. pread, readlink, fstat, send some
arguments and get back an integer return value and some additional
data.  This additional data is called the attachment.

Except, we only get the attachment if the command completes
successfully.  For example, calling readlink with a non existent path
will result in a return packet: 'F-1,2' with no attachment.  This is
as expected.

Within remote_hostio_send_command we call remote_hostio_parse_result,
this parses the status code (-1 in our example above) and
then parses the errno value (2 in our example above).

Back in remote_hostio_parse_result we then hit this block:

  /* Make sure we saw an attachment if and only if we expected one.  */
  if ((attachment_tmp == NULL && attachment != NULL)
      || (attachment_tmp != NULL && attachment == NULL))
    {
      *remote_errno = FILEIO_EINVAL;
      return -1;
    }

Which ensures that commands that expect an attachment, got an
attachment.

The problem is, we'll only get an attachment if the command
succeeded.  If it didn't, then there is no attachment, and that is as
expected.

As remote_hostio_parse_result always sets the returned error number to
FILEIO_SUCCESS unless the packet contained an actual error
number (e.g. 2 in our example above), I suggest we should return early
if remote_hostio_parse_result indicates an error packet.

I ran into this issue while working on another patch.  In that patch I
was checking the error code returned by a remote readlink call and
spotted that when I passed an invalid path I got EINVAL instead of
ENOENT.  This patch fixes this issue.

Unfortunately the patch I was working on evolved, and my need to check
the error code went away, and so, right now, I have no way to reveal
this bug.  But I think this is an obviously correct fix, and worth
merging even without a test.
---
 gdb/remote.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/remote.c b/gdb/remote.c
index 349bc8cf005..241803ab2b3 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12663,6 +12663,9 @@ remote_target::remote_hostio_send_command (int command_bytes, int which_packet,
       return -1;
     }
 
+  if (*remote_errno != FILEIO_SUCCESS)
+    return -1;
+
   /* Make sure we saw an attachment if and only if we expected one.  */
   if ((attachment_tmp == NULL && attachment != NULL)
       || (attachment_tmp != NULL && attachment == NULL))
-- 
2.25.4

Re: [PATCH 1/8] gdb/fileio: fix errno for packets where an attachment is expected

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> I ran into this issue while working on another patch.  In that patch I
Andrew> was checking the error code returned by a remote readlink call and
Andrew> spotted that when I passed an invalid path I got EINVAL instead of
Andrew> ENOENT.  This patch fixes this issue.

Andrew> Unfortunately the patch I was working on evolved, and my need to check
Andrew> the error code went away, and so, right now, I have no way to reveal
Andrew> this bug.  But I think this is an obviously correct fix, and worth
Andrew> merging even without a test.

Makes sense to me.  Thanks for the clear explanation.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 1/8] gdb/fileio: fix errno for packets where an attachment is expected

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> I ran into this issue while working on another patch.  In that patch I
> Andrew> was checking the error code returned by a remote readlink call and
> Andrew> spotted that when I passed an invalid path I got EINVAL instead of
> Andrew> ENOENT.  This patch fixes this issue.
>
> Andrew> Unfortunately the patch I was working on evolved, and my need to check
> Andrew> the error code went away, and so, right now, I have no way to reveal
> Andrew> this bug.  But I think this is an obviously correct fix, and worth
> Andrew> merging even without a test.
>
> Makes sense to me.  Thanks for the clear explanation.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew

[PATCH 2/8] gdb: avoid duplicate search in build_id_to_bfd_suffix

[Andrew Burgess]

In build_id_to_bfd_suffix we look in each debug-file-directory for a
file matching the required build-id.

If we don't find one then we add the sysroot and perform the search
again.

However, the default sysroot is 'target:', and for a native target
this just means to search on the local machine.  So by default, if the
debug information is not present, then we end up searching each
location twice.

I think we only need to perform the "with sysroot" check if either:

 1. The sysroot is something other than 'target:'.  If the user has
 set it to '/some/directory' then we should check this sysroot.  Or if
 the user has set it to 'target:/some/other_directory', this is also
 worth checking.

 2. If the sysroot is 'target:', but the target's filesystem is not
 local (i.e. the user is connected to a remote target), then we should
 check using the sysroot as this will be looking on the remote
 machine.

There's no tests for this as the whole point here is that I'm removing
duplicate work.  No test regressions were seen though.

There should be no user visible changes after this commit.
---
 gdb/build-id.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 41667d5e5cf..fe0494ae54e 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -176,9 +176,15 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
 
       /* Try to look under the sysroot as well.  If the sysroot is
 	 "/the/sysroot", it will give
-	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".  */
+	 "/the/sysroot/usr/lib/debug/.build-id/ab/cdef.debug".
 
-      if (!gdb_sysroot.empty ())
+	 If the sysroot is 'target:' and the target filesystem is local to
+	 GDB then 'target:/path/to/check' becomes '/path/to/check' which
+	 we just checked above.  */
+
+      if (!gdb_sysroot.empty ()
+	  && (gdb_sysroot != TARGET_SYSROOT_PREFIX
+	      || !target_filesystem_is_local ()))
 	{
 	  link = gdb_sysroot + link;
 	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
-- 
2.25.4

Re: [PATCH 2/8] gdb: avoid duplicate search in build_id_to_bfd_suffix

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> There's no tests for this as the whole point here is that I'm removing
Andrew> duplicate work.  No test regressions were seen though.

Andrew> There should be no user visible changes after this commit.

Thanks for doing this.  I agree with your analysis here.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 2/8] gdb: avoid duplicate search in build_id_to_bfd_suffix

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> There's no tests for this as the whole point here is that I'm removing
> Andrew> duplicate work.  No test regressions were seen though.
>
> Andrew> There should be no user visible changes after this commit.
>
> Thanks for doing this.  I agree with your analysis here.
>
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew

[PATCH 3/8] gdb: warn of slow remote file reading only after a successful open

[Andrew Burgess]

While working on a later patch in this series, I noticed that GDB
would print the message:

  Reading /path/to/file from remote target...

Even when /path/to/file doesn't exist on the remote target.

GDB does indeed try to open /path/to/file, but I'm not sure we really
need to tell the user unless we actually manage to open the file, and
plan to read content from it.

If we consider how GDB probes for separate debug files, we can attempt
to open multiple possible files, most of them will not exist.  When we
are native debugging we don't bother telling the user about each file
we're checking for, we just announce any file we finally use.

I think it makes sense to do a similar thing for remote files.

So, in remote_target::remote_hostio_open(), I'd like to move the block
of code that prints the above message to after the open call has been
made, and we should only print the message if the open succeeds.

Now GDB only tells the user about files that we actually open and read
from the remote.
---
 gdb/remote.c                                  |  36 +++---
 gdb/testsuite/gdb.server/remote-read-msgs.c   |  22 ++++
 gdb/testsuite/gdb.server/remote-read-msgs.exp | 119 ++++++++++++++++++
 3 files changed, 160 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/remote-read-msgs.c
 create mode 100644 gdb/testsuite/gdb.server/remote-read-msgs.exp

diff --git a/gdb/remote.c b/gdb/remote.c
index 241803ab2b3..4742cdf2606 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12751,7 +12751,24 @@ remote_target::remote_hostio_open (inferior *inf, const char *filename,
   char *p = rs->buf.data ();
   int left = get_remote_packet_size () - 1;
 
-  if (warn_if_slow)
+  if (remote_hostio_set_filesystem (inf, remote_errno) != 0)
+    return -1;
+
+  remote_buffer_add_string (&p, &left, "vFile:open:");
+
+  remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
+			   strlen (filename));
+  remote_buffer_add_string (&p, &left, ",");
+
+  remote_buffer_add_int (&p, &left, flags);
+  remote_buffer_add_string (&p, &left, ",");
+
+  remote_buffer_add_int (&p, &left, mode);
+
+  int res = remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_open,
+					remote_errno, nullptr, nullptr);
+
+  if (warn_if_slow && res != -1)
     {
       static int warning_issued = 0;
 
@@ -12767,22 +12784,7 @@ remote_target::remote_hostio_open (inferior *inf, const char *filename,
 	}
     }
 
-  if (remote_hostio_set_filesystem (inf, remote_errno) != 0)
-    return -1;
-
-  remote_buffer_add_string (&p, &left, "vFile:open:");
-
-  remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
-			   strlen (filename));
-  remote_buffer_add_string (&p, &left, ",");
-
-  remote_buffer_add_int (&p, &left, flags);
-  remote_buffer_add_string (&p, &left, ",");
-
-  remote_buffer_add_int (&p, &left, mode);
-
-  return remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_open,
-				     remote_errno, NULL, NULL);
+  return res;
 }
 
 int
diff --git a/gdb/testsuite/gdb.server/remote-read-msgs.c b/gdb/testsuite/gdb.server/remote-read-msgs.c
new file mode 100644
index 00000000000..bbcfb01316e
--- /dev/null
+++ b/gdb/testsuite/gdb.server/remote-read-msgs.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/remote-read-msgs.exp b/gdb/testsuite/gdb.server/remote-read-msgs.exp
new file mode 100644
index 00000000000..d2d659aa365
--- /dev/null
+++ b/gdb/testsuite/gdb.server/remote-read-msgs.exp
@@ -0,0 +1,119 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2024 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/>.
+
+# Setup a number of directories in the debug-file-directory, start gdbserver
+# and connect GDB.  The debug information is downloaded from the last
+# directory in the debug-file-directory.
+#
+# We are checking that GDB reports 'Reading <filename> from remote target'
+# only for the paths that are actually being read, and not for paths that
+# GDB is simply probing for existence.
+
+load_lib gdbserver-support.exp
+
+require allow_gdbserver_tests
+require {!is_remote host}
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile] == -1} {
+    return -1
+}
+
+# Split out BINFILE.debug.  Remove debug from BINFILE.
+if {[gdb_gnu_strip_debug $binfile] != 0} {
+    return -1
+}
+
+# Get the '.build-id/xx/xxx...xxx' part of the filename.
+set build_id_filename [build_id_debug_filename_get $binfile]
+
+set hidden_binfile [standard_output_file "hidden_$testfile"]
+set hidden_debuginfo [standard_output_file "hidden_$testfile.debug"]
+
+# Hide (rename) BINFILE and associated debug information, this should ensure
+# GDB can't find it directly.
+remote_exec build "mv $binfile $hidden_binfile"
+remote_exec build "mv ${binfile}.debug $hidden_debuginfo"
+
+# Helper called from gdb_finish when the 'target' is remote.  Ensure the
+# debug directory we create is deleted.
+proc cleanup_remote_target {} {
+    remote_exec target "rm -fr debug/"
+}
+
+if { ![is_remote target] } {
+    set gdbserver_dir [standard_output_file "gdbserver-dir"]/
+} else {
+    lappend gdb_finish_hooks cleanup_remote_target
+    set gdbserver_dir ""
+}
+
+# Copy files to the target (if needed).
+set target_binfile [gdb_remote_download target $hidden_binfile]
+set target_debuginfo [gdb_remote_download target $hidden_debuginfo]
+
+# Setup the debug information on the target.
+remote_exec target \
+    "mkdir -p ${gdbserver_dir}debug/[file dirname $build_id_filename]"
+remote_exec target \
+    "ln -sf $target_debuginfo ${gdbserver_dir}debug/$build_id_filename"
+
+# Reading debug info from the remote target can take a bit of time, so
+# increase the timeout.
+with_timeout_factor 5 {
+    # Restart GDB.
+    clean_restart
+
+    # Add some dummy entries to the debug-file-directory search list.
+    gdb_test_no_output "set debug-file-directory xxx:yyy:debug"
+
+    # Set the sysroot.
+    gdb_test_no_output "set sysroot target:"
+
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
+
+    # Start gdbserver.  This needs to be done after starting GDB.  When
+    # gdbserver is running local to GDB, start gdbserver in a sub-directory,
+    # this prevents GDB from finding the debug information itself.
+    if { ![is_remote target] } {
+	with_cwd $gdbserver_dir {
+	    set res [gdbserver_start "" $target_binfile]
+	}
+    } else {
+	set res [gdbserver_start "" $target_binfile]
+    }
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    # Connect to gdbserver.  The output will be placed into the global
+    # GDB_TARGET_REMOTE_CMD_MSG, and we'll match against this below.
+    gdb_assert {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] == 0} \
+	"connect to gdbserver"
+
+    gdb_assert { ![regexp "Reading xxx/\[^\r\n\]+ from remote target\\.\\.\\.\r\n" \
+		       $gdb_target_remote_cmd_msg] \
+		     && ![regexp "Reading yyy/\[^\r\n\]+ from remote target\\.\\.\\.\r\n" \
+			      $gdb_target_remote_cmd_msg] } \
+	"check xxx/ and yyy/ are not mentioned"
+
+    gdb_assert { [regexp "Reading debug/[string_to_regexp $build_id_filename] from remote target\\.\\.\\.\r\n" \
+		      $gdb_target_remote_cmd_msg] } \
+	"check debug information is found"
+}
-- 
2.25.4

Re: [PATCH 3/8] gdb: warn of slow remote file reading only after a successful open

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> So, in remote_target::remote_hostio_open(), I'd like to move the block
Andrew> of code that prints the above message to after the open call has been
Andrew> made, and we should only print the message if the open succeeds.

Andrew> Now GDB only tells the user about files that we actually open and read
Andrew> from the remote.

Makes sense to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 3/8] gdb: warn of slow remote file reading only after a successful open

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> So, in remote_target::remote_hostio_open(), I'd like to move the block
> Andrew> of code that prints the above message to after the open call has been
> Andrew> made, and we should only print the message if the open succeeds.
>
> Andrew> Now GDB only tells the user about files that we actually open and read
> Andrew> from the remote.
>
> Makes sense to me.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew

[PATCH 4/8] gdb: convert separate-debug-file to new(ish) debug scheme

[Andrew Burgess]

Convert 'set/show debug separate-debug-file' to the new debug scheme.
Though I'm not sure if we can really call it "new" any more!
---
 gdb/build-id.c | 38 +++++++++++++-------------------------
 gdb/symfile.c  | 45 ++++++++++++++++-----------------------------
 gdb/symfile.h  | 19 +++++++++++++++++++
 3 files changed, 48 insertions(+), 54 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index fe0494ae54e..8ce94f1e7a0 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -81,11 +81,7 @@ static gdb_bfd_ref_ptr
 build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
 			 const bfd_byte *build_id)
 {
-  if (separate_debug_file_debug)
-    {
-      gdb_printf (gdb_stdlog, _("  Trying %s..."), link.c_str ());
-      gdb_flush (gdb_stdlog);
-    }
+  separate_debug_file_debug_printf ("Trying %s...", link.c_str ());
 
   /* lrealpath() is expensive even for the usually non-existent files.  */
   gdb::unique_xmalloc_ptr<char> filename_holder;
@@ -100,10 +96,7 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
 
   if (filename == NULL)
     {
-      if (separate_debug_file_debug)
-	gdb_printf (gdb_stdlog,
-		    _(" no, unable to compute real path\n"));
-
+      separate_debug_file_debug_printf ("unable to compute real path");
       return {};
     }
 
@@ -112,23 +105,17 @@ build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
 
   if (debug_bfd == NULL)
     {
-      if (separate_debug_file_debug)
-	gdb_printf (gdb_stdlog, _(" no, unable to open.\n"));
-
+      separate_debug_file_debug_printf ("unable to open.");
       return {};
     }
 
   if (!build_id_verify (debug_bfd.get(), build_id_len, build_id))
     {
-      if (separate_debug_file_debug)
-	gdb_printf (gdb_stdlog, _(" no, build-id does not match.\n"));
-
+      separate_debug_file_debug_printf ("build-id does not match.");
       return {};
     }
 
-  if (separate_debug_file_debug)
-    gdb_printf (gdb_stdlog, _(" yes!\n"));
-
+  separate_debug_file_debug_printf ("found a match");
   return debug_bfd;
 }
 
@@ -140,6 +127,8 @@ static gdb_bfd_ref_ptr
 build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
 			const char *suffix)
 {
+  SEPARATE_DEBUG_FILE_SCOPED_DEBUG_ENTER_EXIT;
+
   /* Keep backward compatibility so that DEBUG_FILE_DIRECTORY being "" will
      cause "/.build-id/..." lookups.  */
 
@@ -223,10 +212,9 @@ find_separate_debug_file_by_buildid (struct objfile *objfile,
   build_id = build_id_bfd_get (objfile->obfd.get ());
   if (build_id != NULL)
     {
-      if (separate_debug_file_debug)
-	gdb_printf (gdb_stdlog,
-		    _("\nLooking for separate debug info (build-id) for "
-		      "%s\n"), objfile_name (objfile));
+      SEPARATE_DEBUG_FILE_SCOPED_DEBUG_START_END
+	("looking for separate debug info (build-id) for %s",
+	 objfile_name (objfile));
 
       gdb_bfd_ref_ptr abfd (build_id_to_debug_bfd (build_id->size,
 						   build_id->data));
@@ -235,9 +223,9 @@ find_separate_debug_file_by_buildid (struct objfile *objfile,
 	  && filename_cmp (bfd_get_filename (abfd.get ()),
 			   objfile_name (objfile)) == 0)
 	{
-	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "\"%s\": separate debug info file has no "
-			"debug info", bfd_get_filename (abfd.get ()));
+	  separate_debug_file_debug_printf
+	    ("\"%s\": separate debug info file has no debug info",
+	     bfd_get_filename (abfd.get ()));
 	  warnings->warn (_("\"%ps\": separate debug info file has no "
 			    "debug info"),
 			  styled_string (file_name_style.style (),
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f7f5be5a39a..2cb1f652867 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1229,6 +1229,8 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 			    struct objfile *parent_objfile,
 			    deferred_warnings *warnings)
 {
+  SEPARATE_DEBUG_FILE_SCOPED_DEBUG_ENTER_EXIT;
+
   unsigned long file_crc;
   int file_crc_p;
   struct stat parent_stat, abfd_stat;
@@ -1243,19 +1245,13 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
   if (filename_cmp (name.c_str (), objfile_name (parent_objfile)) == 0)
     return 0;
 
-  if (separate_debug_file_debug)
-    {
-      gdb_printf (gdb_stdlog, _("  Trying %s..."), name.c_str ());
-      gdb_flush (gdb_stdlog);
-    }
+  separate_debug_file_debug_printf ("Trying %s...", name.c_str ());
 
   gdb_bfd_ref_ptr abfd (gdb_bfd_open (name.c_str (), gnutarget));
 
   if (abfd == NULL)
     {
-      if (separate_debug_file_debug)
-	gdb_printf (gdb_stdlog, _(" no, unable to open.\n"));
-
+      separate_debug_file_debug_printf ("unable to open file");
       return 0;
     }
 
@@ -1277,10 +1273,7 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
       if (abfd_stat.st_dev == parent_stat.st_dev
 	  && abfd_stat.st_ino == parent_stat.st_ino)
 	{
-	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog,
-			_(" no, same file as the objfile.\n"));
-
+	  separate_debug_file_debug_printf ("same file as the objfile");
 	  return 0;
 	}
       verified_as_different = 1;
@@ -1292,9 +1285,7 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 
   if (!file_crc_p)
     {
-      if (separate_debug_file_debug)
-	gdb_printf (gdb_stdlog, _(" no, error computing CRC.\n"));
-
+      separate_debug_file_debug_printf ("error computing CRC");
       return 0;
     }
 
@@ -1310,20 +1301,18 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 	{
 	  if (!gdb_bfd_crc (parent_objfile->obfd.get (), &parent_crc))
 	    {
-	      if (separate_debug_file_debug)
-		gdb_printf (gdb_stdlog,
-			    _(" no, error computing CRC.\n"));
-
+	      separate_debug_file_debug_printf ("error computing CRC");
 	      return 0;
 	    }
 	}
 
       if (verified_as_different || parent_crc != file_crc)
 	{
-	  if (separate_debug_file_debug)
-	    gdb_printf (gdb_stdlog, "the debug information found in \"%s\""
-			" does not match \"%s\" (CRC mismatch).\n",
-			name.c_str (), objfile_name (parent_objfile));
+	  separate_debug_file_debug_printf
+	    ("the debug information found in \"%s\" does not match "
+	     "\"%s\" (CRC mismatch).", name.c_str (),
+	     objfile_name (parent_objfile));
+
 	  warnings->warn (_("the debug information found in \"%ps\""
 			    " does not match \"%ps\" (CRC mismatch)."),
 			  styled_string (file_name_style.style (),
@@ -1335,8 +1324,7 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
       return 0;
     }
 
-  if (separate_debug_file_debug)
-    gdb_printf (gdb_stdlog, _(" yes!\n"));
+  separate_debug_file_debug_printf ("found a match");
 
   return 1;
 }
@@ -1377,10 +1365,9 @@ find_separate_debug_file (const char *dir,
 			  unsigned long crc32, struct objfile *objfile,
 			  deferred_warnings *warnings)
 {
-  if (separate_debug_file_debug)
-    gdb_printf (gdb_stdlog,
-		_("\nLooking for separate debug info (debug link) for "
-		  "%s\n"), objfile_name (objfile));
+  SEPARATE_DEBUG_FILE_SCOPED_DEBUG_START_END
+    ("looking for separate debug info (debug link) for %s",
+     objfile_name (objfile));
 
   /* First try in the same directory as the original file.  */
   std::string debugfile = dir;
diff --git a/gdb/symfile.h b/gdb/symfile.h
index a5b0c91469c..508ba48d161 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -371,6 +371,25 @@ extern gdb_bfd_ref_ptr find_separate_debug_file_in_section (struct objfile *);
 
 extern bool separate_debug_file_debug;
 
+/* Print a "separate-debug-file" debug statement.  */
+
+#define separate_debug_file_debug_printf(fmt, ...)		\
+  debug_prefixed_printf_cond (separate_debug_file_debug,	\
+			      "separate-debug-file",		\
+			      fmt, ##__VA_ARGS__)
+
+/* Print "separate-debug-file" enter/exit debug statements.  */
+
+#define SEPARATE_DEBUG_FILE_SCOPED_DEBUG_ENTER_EXIT \
+  scoped_debug_enter_exit (separate_debug_file_debug,	\
+			   "separate-debug-file")
+
+/* Print "separate-debug-file" start/end debug statements.  */
+
+#define SEPARATE_DEBUG_FILE_SCOPED_DEBUG_START_END(fmt, ...) \
+  scoped_debug_start_end (separate_debug_file_debug,	     \
+			  "separate-debug-file", fmt, ##__VA_ARGS__)
+
 /* Read full symbols immediately.  */
 
 extern int readnow_symbol_files;
-- 
2.25.4

Re: [PATCH 4/8] gdb: convert separate-debug-file to new(ish) debug scheme

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Convert 'set/show debug separate-debug-file' to the new debug scheme.

Looks good.  Thanks.
Approved-By: Tom Tromey <tom@tromey.com>

Andrew> Though I'm not sure if we can really call it "new" any more!

There's ample precedent https://en.wikipedia.org/wiki/Pont_Neuf

Tom

Re: [PATCH 4/8] gdb: convert separate-debug-file to new(ish) debug scheme

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> Convert 'set/show debug separate-debug-file' to the new debug scheme.
>
> Looks good.  Thanks.
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed.

Thanks,
Andrew

[PATCH 5/8] gdb: add target_fileio_stat, but no implementations yet

[Andrew Burgess]

In a later commit I want target_fileio_stat, that is a call that
operates on a filename rather than an open file descriptor as
target_fileio_fstat does.

This commit adds the initial framework for target_fileio_stat, I've
added the top level target function and the virtual target_ops methods
in the target_ops base class.

At this point no actual targets override target_ops::fileio_stat, so
any attempts to call this function will return ENOSYS error code.
---
 gdb/target.c | 31 +++++++++++++++++++++++++++++++
 gdb/target.h | 16 ++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/gdb/target.c b/gdb/target.c
index 276e215945e..37c907830d6 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3198,6 +3198,14 @@ target_ops::fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno)
   return -1;
 }
 
+int
+target_ops::fileio_stat (struct inferior *inf, const char *filename,
+			 struct stat *sb, fileio_error *target_errno)
+{
+  *target_errno = FILEIO_ENOSYS;
+  return -1;
+}
+
 int
 target_ops::fileio_close (int fd, fileio_error *target_errno)
 {
@@ -3317,6 +3325,29 @@ target_fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno)
 
 /* See target.h.  */
 
+int
+target_fileio_stat (struct inferior *inf, const char *filename,
+		    struct stat *sb, fileio_error *target_errno)
+{
+  for (target_ops *t = default_fileio_target (); t != NULL; t = t->beneath ())
+    {
+      int ret = t->fileio_stat (inf, filename, sb, target_errno);
+
+      if (ret == -1 && *target_errno == FILEIO_ENOSYS)
+	continue;
+
+      target_debug_printf_nofunc ("target_fileio_stat (%s) = %d (%d)",
+				  filename, ret,
+				  ret != -1 ? 0 : *target_errno);
+      return ret;
+    }
+
+  *target_errno = FILEIO_ENOSYS;
+  return -1;
+}
+
+/* See target.h.  */
+
 int
 target_fileio_close (int fd, fileio_error *target_errno)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 81de4a678c3..f754f4abd53 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1011,6 +1011,14 @@ struct target_ops
        *TARGET_ERRNO).  */
     virtual int fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno);
 
+    /* Get information about the file FILENAME and put it in SB.  Look for
+       FILENAME in the filesystem as seen by INF.  If INF is NULL, use the
+       filesystem seen by the debugger (GDB or, for remote targets, the
+       remote stub).  Return 0 on success, or -1 if an error occurs (and
+       set *TARGET_ERRNO).  */
+    virtual int fileio_stat (struct inferior *inf, const char *filename,
+			     struct stat *sb, fileio_error *target_errno);
+
     /* Close FD on the target.  Return 0, or -1 if an error occurs
        (and set *TARGET_ERRNO).  */
     virtual int fileio_close (int fd, fileio_error *target_errno);
@@ -2220,6 +2228,14 @@ extern int target_fileio_pread (int fd, gdb_byte *read_buf, int len,
 extern int target_fileio_fstat (int fd, struct stat *sb,
 				fileio_error *target_errno);
 
+/* Get information about the file at FILENAME on the target and put it in
+   SB.  Look in the filesystem as seen by INF.  If INF is NULL, use the
+   filesystem seen by the debugger (GDB or, for remote targets, the remote
+   stub).  Return 0 on success, or -1 if an error occurs (and set
+   *TARGET_ERRNO).  */
+extern int target_fileio_stat (struct inferior *inf, const char *filename,
+			       struct stat *sb, fileio_error *target_errno);
+
 /* Close FD on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
 extern int target_fileio_close (int fd, fileio_error *target_errno);
-- 
2.25.4

[PATCH 6/8] gdb: add GDB side target_ops::fileio_stat implementation

[Andrew Burgess]

This commit adds the GDB side of target_ops::fileio_stat.  There's an
implementation for inf_child_target, which just calls 'lstat', and
there's an implementation for remote_target, which sends a new
vFile:stat packet.

The new packet is documented.

There's still no users of target_fileio_stat as I have not yet added
support for vFile::stat to gdbserver.  If these packets are currently
sent to gdbserver then they will be reported as not supported and the
ENOSYS error code will be returned.
---
 gdb/NEWS            |  5 +++
 gdb/doc/gdb.texinfo | 11 ++++++
 gdb/inf-child.c     | 15 ++++++++
 gdb/inf-child.h     |  2 ++
 gdb/remote.c        | 83 +++++++++++++++++++++++++++++++++++++++------
 5 files changed, 105 insertions(+), 11 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index bb7c4a6a6a6..fb2ecb79205 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -219,6 +219,11 @@ qIsAddressTagged
   file is about, this new packet provides a more generic way to perform such
   a check.
 
+vFile:stat
+  Return information about files on the remote system.  Like
+  vFile:fstat but takes a filename rather than an open file
+  descriptor.
+
 *** Changes in GDB 14
 
 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 750f368f980..d39b7268eba 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24534,6 +24534,10 @@
 @tab @code{vFile:fstat}
 @tab Host I/O
 
+@item @code{hostio-stat-packet}
+@tab @code{vFile:stat}
+@tab Host I/O
+
 @item @code{hostio-setfs-packet}
 @tab @code{vFile:setfs}
 @tab Host I/O
@@ -46299,6 +46303,13 @@
 If an error occurs the return value is -1.  The format of the
 returned binary attachment is as described in @ref{struct stat}.
 
+@item vFile:stat: @var{filename}
+Get information about the file @var{filename} on the target.
+On success the information is returned as a binary attachment
+and the return value is the size of this attachment in bytes.
+If an error occurs the return value is -1.  The format of the
+returned binary attachment is as described in @ref{struct stat}.
+
 @item vFile:unlink: @var{filename}
 Delete the file at @var{filename} on the target.  Return 0,
 or -1 if an error occurs.  The @var{filename} is a string.
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 1318d6b041e..df993b624dd 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -320,6 +320,21 @@ inf_child_target::fileio_fstat (int fd, struct stat *sb, fileio_error *target_er
   return ret;
 }
 
+/* Implementation of to_fileio_stat.  */
+
+int
+inf_child_target::fileio_stat (struct inferior *inf, const char *filename,
+			       struct stat *sb, fileio_error *target_errno)
+{
+  int ret;
+
+  ret = lstat (filename, sb);
+  if (ret == -1)
+    *target_errno = host_to_fileio_error (errno);
+
+  return ret;
+}
+
 /* Implementation of to_fileio_close.  */
 
 int
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index 91955a64f4c..65d42e135ce 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -81,6 +81,8 @@ class inf_child_target
   int fileio_pread (int fd, gdb_byte *read_buf, int len,
 		    ULONGEST offset, fileio_error *target_errno) override;
   int fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno) override;
+  int fileio_stat (struct inferior *inf, const char *filename,
+		   struct stat *sb, fileio_error *target_errno) override;
   int fileio_close (int fd, fileio_error *target_errno) override;
   int fileio_unlink (struct inferior *inf,
 		     const char *filename,
diff --git a/gdb/remote.c b/gdb/remote.c
index 4742cdf2606..7a2fc758721 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -248,6 +248,7 @@ enum {
   PACKET_vFile_unlink,
   PACKET_vFile_readlink,
   PACKET_vFile_fstat,
+  PACKET_vFile_stat,
   PACKET_qXfer_auxv,
   PACKET_qXfer_features,
   PACKET_qXfer_exec_file,
@@ -998,6 +999,9 @@ class remote_target : public process_stratum_target
 
   int fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno) override;
 
+  int fileio_stat (struct inferior *inf, const char *filename,
+		   struct stat *sb, fileio_error *target_errno) override;
+
   int fileio_close (int fd, fileio_error *target_errno) override;
 
   int fileio_unlink (struct inferior *inf,
@@ -13034,6 +13038,41 @@ remote_target::fileio_readlink (struct inferior *inf, const char *filename,
   return ret;
 }
 
+/* Helper function to handle ::fileio_fstat and ::fileio_stat result
+   processing.  When this function is called the remote syscall has been
+   performed and we know we didn't get an error back.
+
+   ATTACHMENT and ATTACHMENT_LEN are the attachment data extracted from the
+   remote syscall reply.  EXPECTED_LEN is the length returned from the
+   fstat or stat call, this the length of the returned data (in ATTACHMENT)
+   once it has been decoded.  The fstat/stat result (from the ATTACHMENT
+   data) is to be placed in ST.  */
+
+static int
+fileio_process_fstat_and_stat_reply (const char *attachment,
+				     int attachment_len,
+				     int expected_len,
+				     struct stat *st)
+{
+  struct fio_stat fst;
+
+  int read_len
+    = remote_unescape_input ((gdb_byte *) attachment, attachment_len,
+			     (gdb_byte *) &fst, sizeof (fst));
+
+  if (read_len != expected_len)
+    error (_("vFile:fstat returned %d, but %d bytes."),
+	   expected_len, read_len);
+
+  if (read_len != sizeof (fst))
+    error (_("vFile:fstat returned %d bytes, but expecting %d."),
+	   read_len, (int) sizeof (fst));
+
+  remote_fileio_to_host_stat (&fst, st);
+
+  return 0;
+}
+
 /* Implementation of to_fileio_fstat.  */
 
 int
@@ -13044,8 +13083,6 @@ remote_target::fileio_fstat (int fd, struct stat *st, fileio_error *remote_errno
   int left = get_remote_packet_size ();
   int attachment_len, ret;
   const char *attachment;
-  struct fio_stat fst;
-  int read_len;
 
   remote_buffer_add_string (&p, &left, "vFile:fstat:");
 
@@ -13077,19 +13114,41 @@ remote_target::fileio_fstat (int fd, struct stat *st, fileio_error *remote_errno
       return 0;
     }
 
-  read_len = remote_unescape_input ((gdb_byte *) attachment, attachment_len,
-				    (gdb_byte *) &fst, sizeof (fst));
+  return fileio_process_fstat_and_stat_reply (attachment, attachment_len,
+					      ret, st);
+}
 
-  if (read_len != ret)
-    error (_("vFile:fstat returned %d, but %d bytes."), ret, read_len);
+/* Implementation of to_fileio_stat.  */
 
-  if (read_len != sizeof (fst))
-    error (_("vFile:fstat returned %d bytes, but expecting %d."),
-	   read_len, (int) sizeof (fst));
+int
+remote_target::fileio_stat (struct inferior *inf, const char *filename,
+			    struct stat *st, fileio_error *remote_errno)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf.data ();
+  int left = get_remote_packet_size () - 1;
 
-  remote_fileio_to_host_stat (&fst, st);
+  if (remote_hostio_set_filesystem (inf, remote_errno) != 0)
+    return {};
 
-  return 0;
+  remote_buffer_add_string (&p, &left, "vFile:stat:");
+
+  remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
+			   strlen (filename));
+
+  int attachment_len;
+  const char *attachment;
+  int ret = remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_stat,
+					remote_errno, &attachment,
+					&attachment_len);
+
+  /* Unlike ::fileio_fstat, the stat fileio call was added later on, and
+     has none of the legacy bfd issues, so we can just return the error.  */
+  if (ret < 0)
+    return ret;
+
+  return fileio_process_fstat_and_stat_reply (attachment, attachment_len,
+					      ret, st);
 }
 
 /* Implementation of to_filesystem_is_local.  */
@@ -16167,6 +16226,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
 
   add_packet_config_cmd (PACKET_vFile_fstat, "vFile:fstat", "hostio-fstat", 0);
 
+  add_packet_config_cmd (PACKET_vFile_stat, "vFile:stat", "hostio-stat", 0);
+
   add_packet_config_cmd (PACKET_vAttach, "vAttach", "attach", 0);
 
   add_packet_config_cmd (PACKET_vRun, "vRun", "run", 0);
-- 
2.25.4

Re: [PATCH 6/8] gdb: add GDB side target_ops::fileio_stat implementation

[Eli Zaretskii]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>
> Date: Wed,  5 Jun 2024 14:15:13 +0100
> 
> This commit adds the GDB side of target_ops::fileio_stat.  There's an
> implementation for inf_child_target, which just calls 'lstat', and
> there's an implementation for remote_target, which sends a new
> vFile:stat packet.
> 
> The new packet is documented.
> 
> There's still no users of target_fileio_stat as I have not yet added
> support for vFile::stat to gdbserver.  If these packets are currently
> sent to gdbserver then they will be reported as not supported and the
> ENOSYS error code will be returned.
> ---
>  gdb/NEWS            |  5 +++
>  gdb/doc/gdb.texinfo | 11 ++++++
>  gdb/inf-child.c     | 15 ++++++++
>  gdb/inf-child.h     |  2 ++
>  gdb/remote.c        | 83 +++++++++++++++++++++++++++++++++++++++------
>  5 files changed, 105 insertions(+), 11 deletions(-)

Thanks, the documentation parts are okay.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

[PATCH 7/8] gdbserver: add gdbserver support for vFile::stat packet

[Andrew Burgess]

After the previous two commits, this commit adds support for the
vFile::stat packet to gdbserver.  This is pretty similar to the
handling for vFile::fstat, but instead calls 'lstat'.

There's still no users of target_fileio_stat in GDB, that will come in
a later commit.
---
 gdbserver/hostio.cc | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
index c5ae35ab7dd..cc47d6869d3 100644
--- a/gdbserver/hostio.cc
+++ b/gdbserver/hostio.cc
@@ -486,6 +486,42 @@ handle_fstat (char *own_buf, int *new_packet_len)
     write_enn (own_buf);
 }
 
+static void
+handle_stat (char *own_buf, int *new_packet_len)
+{
+  int bytes_sent;
+  char *p;
+  struct stat st;
+  struct fio_stat fst;
+  char filename[HOSTIO_PATH_MAX];
+
+  p = own_buf + strlen ("vFile:stat:");
+
+  if (require_filename (&p, filename)
+      || require_end (p))
+    {
+      hostio_packet_error (own_buf);
+      return;
+    }
+
+  if (lstat (filename, &st) == -1)
+    {
+      hostio_error (own_buf);
+      return;
+    }
+
+  host_to_fileio_stat (&st, &fst);
+
+  bytes_sent = hostio_reply_with_data (own_buf,
+				       (char *) &fst, sizeof (fst),
+				       new_packet_len);
+
+  /* If the response does not fit into a single packet, do not attempt
+     to return a partial response, but simply fail.  */
+  if (bytes_sent < sizeof (fst))
+    write_enn (own_buf);
+}
+
 static void
 handle_close (char *own_buf)
 {
@@ -603,6 +639,8 @@ handle_vFile (char *own_buf, int packet_len, int *new_packet_len)
     handle_pwrite (own_buf, packet_len);
   else if (startswith (own_buf, "vFile:fstat:"))
     handle_fstat (own_buf, new_packet_len);
+  else if (startswith (own_buf, "vFile:stat:"))
+    handle_stat (own_buf, new_packet_len);
   else if (startswith (own_buf, "vFile:close:"))
     handle_close (own_buf);
   else if (startswith (own_buf, "vFile:unlink:"))
-- 
2.25.4

[PATCH 8/8] gdb: check for multiple matching build-id files

[Andrew Burgess]

Within the debug-file-directory GDB looks for the existence of a
.build-id directory.

Within the .build-id directory GDB looks for files with the form:

  .build-id/ff/4b4142d62b399499844924d53e33d4028380db.debug

which contain the debug information for the objfile with the build-id
ff4b4142d62b399499844924d53e33d4028380db.

There appear to be two strategies for populating the .build-id
directory.  Ubuntu takes the approach of placing the actual debug
information in this directory, so
4b4142d62b399499844924d53e33d4028380db.debug is an actual file
containing the debug information.

Fedora, RHEL, and SUSE take a slightly different approach, placing the
debug information elsewhere, and then creating symlinks in the
.build-id directory back to the original debug information file.  The
actual debug information is arranged in a mirror of the filesystem
within the debug directory, as an example, if the debug-file-directory
is /usr/lib/debug, then the debug information for /bin/foo can be
found in /usr/lib/debug/bin/foo.debug.

Where this gets interesting is that in some cases a package will
install a single binary with multiple names, in this case a single
binary will be install with either hard-links, or symlinks providing
the alternative names.

The debug information for these multiple binaries will then be placed
into the /usr/lib/debug/ tree, and again, links are created so a
single file can provide debug information for each of the names that
binary presents as.

In the .build-id tree though we have a problem.  Do we have a single
entry that links to one of the binary names?  This would work; a user
debugging any of the binaries will find the debug information based on
the build-id, and will get the correct information, after all the
binaries are identical (same file linked together).  But there is one
problem with this approach.

Sometimes, for *reasons* it's possible that one or more the linked
binaries might get removed.  I'm honestly not 100% certain under what
circumstances this can happen, but what I observe is that sometime a
single name for a binary, and its corresponding .debug entry, can be
removed.  If this happens to be the entry that the .build-id link is
pointing at, then we have a problem.  The user can no longer find the
debug information based on the .build-id.

The solution that Fedora, RHEL, & SUSE have adopted is to add multiple
entries in the .build-id tree, with each entry pointing to a different
name within the debug/ tree, a sequence number is added to the
build-id to distinguish the multiple entries.  Thus, we might end up
with a layout like this:

  /bin/
    binary_name_1
    binary_name_2
    binary_name_3
  /usr/
    lib/
      debug/
        bin/
	  binary_name_1.debug
	  binary_name_2.debug -> binary_name_1.debug	[ HARDLINK ]
	  binary_name_3.debug -> binary_name_1.debug    [ HARDLINK ]
        .build-id/
	  a3/
	    4b4142d62b399499844924d53e33d4028380db.debug -> ../../../debug/bin/binary_name_1.debug	[ SYMLINK ]
	    4b4142d62b399499844924d53e33d4028380db.1.debug -> ../../../debug/bin/binary_name_2.debug	[ SYMLINK ]
	    4b4142d62b399499844924d53e33d4028380db.2.debug -> ../../../debug/bin/binary_name_3.debug	[ SYMLINK ]

With current master GDB, debug information will only ever be looked up
via the 4b4142d62b399499844924d53e33d4028380db.debug link.  But if
binary_name_1 and its corresponding binary_name_1.debug are ever
removed, then master GDB will fail to find the debug information.

Ubuntu seems to have a much better approach for debug information
handling; they place the debug information directly into the .build-id
tree, so there only ever needs to be a single entry for any one
build-id.  It's not clear if/how they handle the case where multiple
names might share a single .debug file, if one of those names is then
uninstalled, how do they know the .debug file should be retained or
not ... but I assume that problem either doesn't exist or has been
solved.

Anyway, for a while Fedora has carried a patch that handles the
build-id sequence number logic, and I'd like to try and get this patch
upstream.

I'm aware that this is a patch that applies to only some (probably a
minority) of distros.  However, the logic is contained to only a
single function in build-id.c, and isn't that complex, so I'm hoping
that there wont be too many objections.

For distros that don't have build-id sequence numbers then they should
see no change.  The unnumbered file is checked for first, and only if
that exists, but isn't readable, do we check for numbered files
instead.

Tests are included.
---
 gdb/build-id.c                              | 175 +++++++++++++----
 gdb/testsuite/gdb.base/build-id-seqno.c     |  22 +++
 gdb/testsuite/gdb.base/build-id-seqno.exp   | 133 +++++++++++++
 gdb/testsuite/gdb.server/build-id-seqno.c   |  22 +++
 gdb/testsuite/gdb.server/build-id-seqno.exp | 198 ++++++++++++++++++++
 5 files changed, 512 insertions(+), 38 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.exp
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.exp

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 8ce94f1e7a0..1469222ca49 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -73,50 +73,150 @@ build_id_verify (bfd *abfd, size_t check_len, const bfd_byte *check)
   return retval;
 }
 
-/* Helper for build_id_to_debug_bfd.  LINK is a path to a potential
-   build-id-based separate debug file, potentially a symlink to the real file.
-   If the file exists and matches BUILD_ID, return a BFD reference to it.  */
+/* Helper for build_id_to_debug_bfd.  ORIGINAL_LINK with SUFFIX appended is
+   a path to a potential build-id-based separate debug file, potentially a
+   symlink to the real file.  If the file exists and matches BUILD_ID,
+   return a BFD reference to it.  */
 
 static gdb_bfd_ref_ptr
-build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
-			 const bfd_byte *build_id)
+build_id_to_debug_bfd_1 (const std::string &original_link,
+			 size_t build_id_len, const bfd_byte *build_id,
+			 const char *suffix)
 {
-  separate_debug_file_debug_printf ("Trying %s...", link.c_str ());
-
-  /* lrealpath() is expensive even for the usually non-existent files.  */
-  gdb::unique_xmalloc_ptr<char> filename_holder;
-  const char *filename = nullptr;
-  if (is_target_filename (link))
-    filename = link.c_str ();
-  else if (access (link.c_str (), F_OK) == 0)
+  tribool supports_target_stat = TRIBOOL_UNKNOWN;
+
+  /* Drop the 'target:' prefix if the target filesystem is local.  */
+  std::string_view original_link_view (original_link);
+  if (is_target_filename (original_link) && target_filesystem_is_local ())
+    original_link_view
+      = original_link_view.substr (strlen (TARGET_SYSROOT_PREFIX));
+
+  /* The upper bound of '10' here is completely arbitrary.  The loop should
+     terminate via 'break' when either (a) a readable symlink is found, or
+     (b) a non-existing entry is found.
+
+     However, for remote targets, we rely on the remote returning sane
+     error codes.  If a remote sends back the wrong error code then it
+     might trick GDB into thinking that the symlink exists, but points to a
+     missing file, in which case GDB will try the next seqno.  We don't
+     want a broken remote to cause GDB to spin here forever, hence a fixed
+     upper bound.  */
+
+  for (unsigned seqno = 0; seqno < 10; seqno++)
     {
-      filename_holder.reset (lrealpath (link.c_str ()));
-      filename = filename_holder.get ();
-    }
+      std::string link (original_link_view);
 
-  if (filename == NULL)
-    {
-      separate_debug_file_debug_printf ("unable to compute real path");
-      return {};
-    }
+      if (seqno > 0)
+	string_appendf (link, ".%u", seqno);
 
-  /* We expect to be silent on the non-existing files.  */
-  gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename, gnutarget);
+      link += suffix;
 
-  if (debug_bfd == NULL)
-    {
-      separate_debug_file_debug_printf ("unable to open.");
-      return {};
-    }
+      separate_debug_file_debug_printf ("Trying %s...", link.c_str ());
 
-  if (!build_id_verify (debug_bfd.get(), build_id_len, build_id))
-    {
-      separate_debug_file_debug_printf ("build-id does not match.");
-      return {};
+      gdb::unique_xmalloc_ptr<char> filename_holder;
+      const char *filename = nullptr;
+      if (is_target_filename (link))
+	{
+	  gdb_assert (link.length () >= strlen (TARGET_SYSROOT_PREFIX));
+	  const char *link_on_target
+	    = link.c_str () + strlen (TARGET_SYSROOT_PREFIX);
+
+	  fileio_error target_errno;
+	  if (supports_target_stat != TRIBOOL_FALSE)
+	    {
+	      struct stat sb;
+	      int res = target_fileio_stat (nullptr, link_on_target, &sb,
+					    &target_errno);
+
+	      if (res != 0 && target_errno != FILEIO_ENOSYS)
+		{
+		  supports_target_stat = TRIBOOL_TRUE;
+		  separate_debug_file_debug_printf ("path doesn't exist");
+		  break;
+		}
+	      else if (res != 0 && target_errno == FILEIO_ENOSYS)
+		supports_target_stat = TRIBOOL_FALSE;
+	      else
+		{
+		  supports_target_stat = TRIBOOL_TRUE;
+		  filename = link.c_str ();
+		}
+	    }
+
+	  if (supports_target_stat == TRIBOOL_FALSE)
+	    {
+	      gdb_assert (filename == nullptr);
+
+	      /* Connecting to a target that doesn't support 'stat'.  Try
+		 'readlink' as an alternative.  This isn't ideal, but is
+		 maybe better than nothing.  Returns EINVAL if the path
+		 isn't a symbolic link, which hints that the path is
+		 available -- there are other errors e.g. ENOENT for when
+		 the path doesn't exist, but we just assume that anything
+		 other than EINVAL indicates the path doesn't exist.  */
+	      std::optional<std::string> link_target
+		= target_fileio_readlink (nullptr, link_on_target,
+					  &target_errno);
+	      if (link_target.has_value ()
+		  || target_errno == FILEIO_EINVAL)
+		filename = link.c_str ();
+	      else
+		{
+		  separate_debug_file_debug_printf ("path doesn't exist");
+		  break;
+		}
+	    }
+	}
+      else
+	{
+	  struct stat buf;
+
+	  /* The `access' call below automatically dereferences LINK, but
+	     we want to stop incrementing SEQNO once we find a symlink
+	     that doesn't exist.  */
+	  if (lstat (link.c_str (), &buf) != 0)
+	    {
+	      separate_debug_file_debug_printf ("path doesn't exist");
+	      break;
+	    }
+
+	  /* Can LINK be accessed, or if LINK is a symlink, can the file
+	     pointed too be accessed?  Do this as lrealpath() is
+	     expensive, even for the usually non-existent files.  */
+	  if (access (link.c_str (), F_OK) == 0)
+	    {
+	      filename_holder.reset (lrealpath (link.c_str ()));
+	      filename = filename_holder.get ();
+	    }
+	}
+
+      if (filename == nullptr)
+	{
+	  separate_debug_file_debug_printf ("unable to compute real path");
+	  continue;
+	}
+
+      /* We expect to be silent on the non-existing files.  */
+      gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename, gnutarget);
+
+      if (debug_bfd == NULL)
+	{
+	  separate_debug_file_debug_printf ("unable to open `%s`", filename);
+	  continue;
+	}
+
+      if (!build_id_verify (debug_bfd.get(), build_id_len, build_id))
+	{
+	  separate_debug_file_debug_printf ("build-id does not match");
+	  continue;
+	}
+
+      separate_debug_file_debug_printf ("found a match");
+      return debug_bfd;
     }
 
-  separate_debug_file_debug_printf ("found a match");
-  return debug_bfd;
+  separate_debug_file_debug_printf ("no suitable file found");
+  return {};
 }
 
 /* Common code for finding BFDs of a given build-id.  This function
@@ -156,10 +256,8 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
       while (size-- > 0)
 	string_appendf (link, "%02x", (unsigned) *data++);
 
-      link += suffix;
-
       gdb_bfd_ref_ptr debug_bfd
-	= build_id_to_debug_bfd_1 (link, build_id_len, build_id);
+	= build_id_to_debug_bfd_1 (link, build_id_len, build_id, suffix);
       if (debug_bfd != NULL)
 	return debug_bfd;
 
@@ -176,7 +274,8 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
 	      || !target_filesystem_is_local ()))
 	{
 	  link = gdb_sysroot + link;
-	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
+	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id,
+					       suffix);
 	  if (debug_bfd != NULL)
 	    return debug_bfd;
 	}
diff --git a/gdb/testsuite/gdb.base/build-id-seqno.c b/gdb/testsuite/gdb.base/build-id-seqno.c
new file mode 100644
index 00000000000..e2119ba7d92
--- /dev/null
+++ b/gdb/testsuite/gdb.base/build-id-seqno.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/build-id-seqno.exp b/gdb/testsuite/gdb.base/build-id-seqno.exp
new file mode 100644
index 00000000000..b13ac8edd86
--- /dev/null
+++ b/gdb/testsuite/gdb.base/build-id-seqno.exp
@@ -0,0 +1,133 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2024 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/>.
+
+# Setup a .build-id/ based debug directory containing multiple entries
+# for the same build-id, with each entry given a different sequence
+# number.
+#
+# Ensure that GDB will scan over broken symlinks for the same build-id
+# (but different sequence number) to find later working symlinks.
+#
+# This test only places debug information on the host, so it is always
+# local to GDB.
+
+require {!is_remote host}
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile] == -1} {
+    return -1
+}
+
+# Split out BINFILE.debug.  Remove debug from BINFILE.
+if {[gdb_gnu_strip_debug $binfile] != 0} {
+    return -1
+}
+
+# Get the '.build-id/xx/xxx...xxx' part of the filename.
+set build_id_filename [build_id_debug_filename_get $binfile]
+
+# Hide (rename) BINFILE.debug, this should ensure GDB can't find it
+# directly but needs to look for the build-id based file in the debug
+# directory.
+set hidden_debuginfo [standard_output_file "hidden_$testfile.debug"]
+remote_exec build "mv ${binfile}.debug $hidden_debuginfo"
+
+# A filename that doesn't exist.  Some symlinks will point at this
+# file.
+set missing_debuginfo [host_standard_output_file "missing_debuginfo"]
+
+# Create the debug directory, and the .build-id directory structure
+# within it.
+set debugdir [host_standard_output_file "debug"]
+remote_exec host "mkdir -p $debugdir/[file dirname $build_id_filename]"
+
+set host_hidden_debuginfo [gdb_remote_download host $hidden_debuginfo]
+remote_exec host "ln -fs $host_hidden_debuginfo $debugdir/$build_id_filename"
+
+# Start GDB and load global BINFILE.  If FIND_DEBUGINFO is true then
+# we expect GDB to find the debug information matching BINFILE,
+# otherwise, we expect GDB not to find the debug information.
+proc load_binfile_check_debug_is_found { find_debuginfo testname } {
+    with_test_prefix "$testname" {
+	clean_restart
+
+	gdb_test_no_output "set debug-file-directory $::debugdir" \
+	    "set debug-file-directory"
+
+	gdb_file_cmd $::binfile
+
+	if { $find_debuginfo } {
+	    gdb_assert { [regexp [string_to_regexp \
+				      "Reading symbols from $::hidden_debuginfo..."] \
+			      $::gdb_file_cmd_msg] } \
+		"debuginfo was read via build-id"
+	} else {
+	    gdb_assert { [regexp "\\(No debugging symbols found in \[^\r\n\]+/$::testfile\\)" \
+			      $::gdb_file_cmd_msg] } \
+	    }
+    }
+}
+
+# Return a copy of FILENAME, which should end '.debug', with NUMBER
+# added, e.g. add_seqno 1 "foo.debug" --> "foo.1.debug".
+proc add_seqno { number filename } {
+    return [regsub "\.debug\$" $filename ".${number}.debug"]
+}
+
+load_binfile_check_debug_is_found true \
+    "find debuginfo with a single build-id file"
+
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 1 $build_id_filename]"
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 2 $build_id_filename]"
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 3 $build_id_filename]"
+
+load_binfile_check_debug_is_found true \
+    "find debuginfo with 4 build-id files"
+
+remote_exec host "ln -fs $missing_debuginfo $debugdir/$build_id_filename"
+
+load_binfile_check_debug_is_found true \
+    "find debuginfo, first build-id file is bad"
+
+remote_exec host "ln -fs $missing_debuginfo \
+	    	     $debugdir/[add_seqno 1 $build_id_filename]"
+remote_exec host "ln -fs $missing_debuginfo \
+	    	     $debugdir/[add_seqno 3 $build_id_filename]"
+
+load_binfile_check_debug_is_found true  \
+    "find debuginfo, first 2 build-id files are bad"
+
+remote_exec host "ln -fs $missing_debuginfo \
+	    	     $debugdir/[add_seqno 2 $build_id_filename]"
+
+load_binfile_check_debug_is_found false  \
+    "cannot find debuginfo, all build-id files are bad"
+
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 3 $build_id_filename]"
+
+load_binfile_check_debug_is_found true  \
+    "find debuginfo, last build-id file is good"
+
+remote_exec host "rm -f $debugdir/[add_seqno 1 $build_id_filename]"
+
+load_binfile_check_debug_is_found false  \
+    "cannot find debuginfo, file with seqno 1 is missing"
diff --git a/gdb/testsuite/gdb.server/build-id-seqno.c b/gdb/testsuite/gdb.server/build-id-seqno.c
new file mode 100644
index 00000000000..e2119ba7d92
--- /dev/null
+++ b/gdb/testsuite/gdb.server/build-id-seqno.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/build-id-seqno.exp b/gdb/testsuite/gdb.server/build-id-seqno.exp
new file mode 100644
index 00000000000..c565d5a4348
--- /dev/null
+++ b/gdb/testsuite/gdb.server/build-id-seqno.exp
@@ -0,0 +1,198 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2024 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/>.
+
+# Setup a .build-id/ based debug directory containing multiple entries
+# for the same build-id, with each entry given a different sequence
+# number.
+#
+# Ensure that GDB will scan over broken symlinks for the same build-id
+# (but different sequence number) to find later working symlinks.
+#
+# This test places the build-id files within a directory next to where
+# gdbserver is started, and places a relative address in the
+# debug-file-directory, in this way we require GDB to find the debug
+# information via gdbserver.
+
+require {!is_remote host}
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile] == -1} {
+    return -1
+}
+
+# Split out BINFILE.debug.  Remove debug from BINFILE.
+if {[gdb_gnu_strip_debug $binfile] != 0} {
+    return -1
+}
+
+# Get the '.build-id/xx/xxx...xxx' part of the filename.
+set build_id_filename [build_id_debug_filename_get $binfile]
+
+# Hide (rename) BINFILE.debug, this should ensure GDB can't find it
+# directly but needs to look for the build-id based file in the debug
+# directory.
+set hidden_debuginfo [standard_output_file "hidden_$testfile.debug"]
+remote_exec build "mv ${binfile}.debug $hidden_debuginfo"
+
+# A filename that doesn't exist.  Some symlinks will point at this
+# file.
+set missing_debuginfo "missing_debuginfo"
+
+# Helper called from gdb_finish when the 'target' is remote.  Ensure the
+# debug directory we create is deleted.
+proc cleanup_remote_target {} {
+    remote_exec target "rm -fr debug/"
+}
+
+if { ![is_remote target] } {
+    set gdbserver_dir [standard_output_file "gdbserver-dir"]/
+} else {
+    lappend gdb_finish_hooks cleanup_remote_target
+    set gdbserver_dir ""
+}
+
+# Copy files to the target (if needed).
+set target_binfile [gdb_remote_download target $binfile]
+set target_debuginfo [gdb_remote_download target $hidden_debuginfo]
+
+# Setup the debug information on the target.
+set debugdir "${gdbserver_dir}debug"
+remote_exec target \
+    "mkdir -p $debugdir/[file dirname $build_id_filename]"
+remote_exec target \
+    "ln -sf $target_debuginfo $debugdir/$build_id_filename"
+
+# Start GDB and load global BINFILE.  If DEBUGINFO_FILE is not the
+# empty string then this contains the '.build-id/xx/xxx....xxxx' part
+# of the filename which we expect GDB to read from the remote target.
+# If DEBUGINFO_FILE is the empty string then we don't expect GDB to
+# find any debug information.
+proc load_binfile_check_debug_is_found { debuginfo_file testname } {
+    with_test_prefix "$testname" {
+	with_timeout_factor 5 {
+	    # Probing for .build-id based debug files on remote
+	    # targets uses the vFile:stat packet by default, though
+	    # there is a work around that avoids this which can be
+	    # used if GDB is connected to an older gdbserver without
+	    # 'stat' support.
+	    #
+	    # Check the work around works by disabling use of the
+	    # vFile:stat packet.
+	    foreach_with_prefix stat_pkt {auto off} {
+		clean_restart
+
+		gdb_test_no_output "set debug-file-directory debug" \
+		    "set debug-file-directory"
+
+		gdb_test_no_output "set sysroot target:"
+
+		gdb_test "set remote hostio-stat-packet $stat_pkt"
+
+		# Make sure we're disconnected, in case we're testing with an
+		# extended-remote board, therefore already connected.
+		gdb_test "disconnect" ".*"
+
+		# Start gdbserver.  This needs to be done after starting GDB.  When
+		# gdbserver is running local to GDB, start gdbserver in a sub-directory,
+		# this prevents GDB from finding the debug information itself.
+		if { ![is_remote target] } {
+		    with_cwd $::gdbserver_dir {
+			set res [gdbserver_start "" $::target_binfile]
+		    }
+		} else {
+		    set res [gdbserver_start "" $::target_binfile]
+		}
+		set gdbserver_protocol [lindex $res 0]
+		set gdbserver_gdbport [lindex $res 1]
+
+		# Connect to gdbserver.  The output will be placed into the global
+		# GDB_TARGET_REMOTE_CMD_MSG, and we'll match against this below.
+		gdb_assert {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] == 0} \
+		    "connect to gdbserver"
+
+		if { $debuginfo_file ne "" } {
+		    gdb_assert { [regexp "Reading symbols from target:debug/[string_to_regexp $debuginfo_file]\\.\\.\\." \
+				      $::gdb_target_remote_cmd_msg] } \
+			"debuginfo was read via build-id"
+		    gdb_assert { [regexp "Reading debug/[string_to_regexp $debuginfo_file] from remote target\\.\\.\\." \
+				      $::gdb_target_remote_cmd_msg] } \
+			"debuginfo was read from remote target"
+		} else {
+		    gdb_assert { [regexp "\\(No debugging symbols found in \[^\r\n\]+/$::testfile\\)" \
+				      $::gdb_target_remote_cmd_msg] }
+		}
+	    }
+	}
+    }
+}
+
+# Return a copy of FILENAME, which should end '.debug', with NUMBER
+# added, e.g. add_seqno 1 "foo.debug" --> "foo.1.debug".
+proc add_seqno { number filename } {
+    return [regsub "\.debug\$" $filename ".${number}.debug"]
+}
+
+# Precompute sequence numbered build-id filenames.
+set build_id_1_filename [add_seqno 1 $build_id_filename]
+set build_id_2_filename [add_seqno 2 $build_id_filename]
+set build_id_3_filename [add_seqno 3 $build_id_filename]
+
+load_binfile_check_debug_is_found $build_id_filename \
+    "find debuginfo with a single build-id file"
+
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_1_filename"
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_2_filename"
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_3_filename"
+
+load_binfile_check_debug_is_found $build_id_filename \
+    "find debuginfo with 4 build-id files"
+
+remote_exec target "ln -fs $missing_debuginfo $debugdir/$build_id_filename"
+
+load_binfile_check_debug_is_found $build_id_1_filename \
+    "find debuginfo, first build-id file is bad"
+
+remote_exec target "ln -fs $missing_debuginfo \
+	    	     $debugdir/$build_id_1_filename"
+remote_exec target "ln -fs $missing_debuginfo \
+	    	     $debugdir/$build_id_3_filename"
+
+load_binfile_check_debug_is_found $build_id_2_filename  \
+    "find debuginfo, first 2 build-id files are bad"
+
+remote_exec target "ln -fs $missing_debuginfo \
+	    	     $debugdir/$build_id_2_filename"
+
+load_binfile_check_debug_is_found ""  \
+    "cannot find debuginfo, all build-id files are bad"
+
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_3_filename"
+
+load_binfile_check_debug_is_found $build_id_3_filename  \
+    "find debuginfo, last build-id file is good"
+
+remote_exec target "rm -f $debugdir/$build_id_1_filename"
+
+load_binfile_check_debug_is_found ""  \
+    "cannot find debuginfo, file with seqno 1 is missing"
-- 
2.25.4

[PATCH 0/8] Extension for looking up debug info by build-id

[Andrew Burgess]

In v2:

  - First 4 patches from v1 were approved, and have been merged.

  - Reamining 4 patches have been rebased to HEAD of master.  This did
    throw up an issue in sysroot-debug-lookup.exp, but I think the
    test needed updating (see patch #4 for details).

  - Otherise, patches are unchagned from v1.

In v1:

  - Patches #1, #2, & #3 add target_ops::fileio_stat() function.

  - Patch #4 adds an extension to how GDB looks for files based on
      build-id.

---

Andrew Burgess (4):
  gdb: add target_fileio_stat, but no implementations yet
  gdb: add GDB side target_ops::fileio_stat implementation
  gdbserver: add gdbserver support for vFile::stat packet
  gdb: check for multiple matching build-id files

 gdb/NEWS                                      |   5 +
 gdb/build-id.c                                | 175 ++++++++++++----
 gdb/doc/gdb.texinfo                           |  11 +
 gdb/inf-child.c                               |  15 ++
 gdb/inf-child.h                               |   2 +
 gdb/remote.c                                  |  83 +++++++-
 gdb/target.c                                  |  31 +++
 gdb/target.h                                  |  16 ++
 gdb/testsuite/gdb.base/build-id-seqno.c       |  22 ++
 gdb/testsuite/gdb.base/build-id-seqno.exp     | 133 ++++++++++++
 .../gdb.base/sysroot-debug-lookup.exp         |  13 +-
 gdb/testsuite/gdb.server/build-id-seqno.c     |  22 ++
 gdb/testsuite/gdb.server/build-id-seqno.exp   | 198 ++++++++++++++++++
 gdbserver/hostio.cc                           |  38 ++++
 14 files changed, 707 insertions(+), 57 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.exp
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.exp


base-commit: 5a011d5b86bea5af144f7242192b21e18c349142
-- 
2.25.4

[PATCHv2 1/4] gdb: add target_fileio_stat, but no implementations yet

[Andrew Burgess]

In a later commit I want target_fileio_stat, that is a call that
operates on a filename rather than an open file descriptor as
target_fileio_fstat does.

This commit adds the initial framework for target_fileio_stat, I've
added the top level target function and the virtual target_ops methods
in the target_ops base class.

At this point no actual targets override target_ops::fileio_stat, so
any attempts to call this function will return ENOSYS error code.
---
 gdb/target.c | 31 +++++++++++++++++++++++++++++++
 gdb/target.h | 16 ++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/gdb/target.c b/gdb/target.c
index 1b5aa11ed6f..26e9bb0146d 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3198,6 +3198,14 @@ target_ops::fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno)
   return -1;
 }
 
+int
+target_ops::fileio_stat (struct inferior *inf, const char *filename,
+			 struct stat *sb, fileio_error *target_errno)
+{
+  *target_errno = FILEIO_ENOSYS;
+  return -1;
+}
+
 int
 target_ops::fileio_close (int fd, fileio_error *target_errno)
 {
@@ -3317,6 +3325,29 @@ target_fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno)
 
 /* See target.h.  */
 
+int
+target_fileio_stat (struct inferior *inf, const char *filename,
+		    struct stat *sb, fileio_error *target_errno)
+{
+  for (target_ops *t = default_fileio_target (); t != NULL; t = t->beneath ())
+    {
+      int ret = t->fileio_stat (inf, filename, sb, target_errno);
+
+      if (ret == -1 && *target_errno == FILEIO_ENOSYS)
+	continue;
+
+      target_debug_printf_nofunc ("target_fileio_stat (%s) = %d (%d)",
+				  filename, ret,
+				  ret != -1 ? 0 : *target_errno);
+      return ret;
+    }
+
+  *target_errno = FILEIO_ENOSYS;
+  return -1;
+}
+
+/* See target.h.  */
+
 int
 target_fileio_close (int fd, fileio_error *target_errno)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 81de4a678c3..f754f4abd53 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1011,6 +1011,14 @@ struct target_ops
        *TARGET_ERRNO).  */
     virtual int fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno);
 
+    /* Get information about the file FILENAME and put it in SB.  Look for
+       FILENAME in the filesystem as seen by INF.  If INF is NULL, use the
+       filesystem seen by the debugger (GDB or, for remote targets, the
+       remote stub).  Return 0 on success, or -1 if an error occurs (and
+       set *TARGET_ERRNO).  */
+    virtual int fileio_stat (struct inferior *inf, const char *filename,
+			     struct stat *sb, fileio_error *target_errno);
+
     /* Close FD on the target.  Return 0, or -1 if an error occurs
        (and set *TARGET_ERRNO).  */
     virtual int fileio_close (int fd, fileio_error *target_errno);
@@ -2220,6 +2228,14 @@ extern int target_fileio_pread (int fd, gdb_byte *read_buf, int len,
 extern int target_fileio_fstat (int fd, struct stat *sb,
 				fileio_error *target_errno);
 
+/* Get information about the file at FILENAME on the target and put it in
+   SB.  Look in the filesystem as seen by INF.  If INF is NULL, use the
+   filesystem seen by the debugger (GDB or, for remote targets, the remote
+   stub).  Return 0 on success, or -1 if an error occurs (and set
+   *TARGET_ERRNO).  */
+extern int target_fileio_stat (struct inferior *inf, const char *filename,
+			       struct stat *sb, fileio_error *target_errno);
+
 /* Close FD on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
 extern int target_fileio_close (int fd, fileio_error *target_errno);
-- 
2.25.4

[PATCHv2 2/4] gdb: add GDB side target_ops::fileio_stat implementation

[Andrew Burgess]

This commit adds the GDB side of target_ops::fileio_stat.  There's an
implementation for inf_child_target, which just calls 'lstat', and
there's an implementation for remote_target, which sends a new
vFile:stat packet.

The new packet is documented.

There's still no users of target_fileio_stat as I have not yet added
support for vFile::stat to gdbserver.  If these packets are currently
sent to gdbserver then they will be reported as not supported and the
ENOSYS error code will be returned.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS            |  5 +++
 gdb/doc/gdb.texinfo | 11 ++++++
 gdb/inf-child.c     | 15 ++++++++
 gdb/inf-child.h     |  2 ++
 gdb/remote.c        | 83 +++++++++++++++++++++++++++++++++++++++------
 5 files changed, 105 insertions(+), 11 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index bb7c4a6a6a6..fb2ecb79205 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -219,6 +219,11 @@ qIsAddressTagged
   file is about, this new packet provides a more generic way to perform such
   a check.
 
+vFile:stat
+  Return information about files on the remote system.  Like
+  vFile:fstat but takes a filename rather than an open file
+  descriptor.
+
 *** Changes in GDB 14
 
 * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 86cd420832a..c55913c7c79 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24534,6 +24534,10 @@
 @tab @code{vFile:fstat}
 @tab Host I/O
 
+@item @code{hostio-stat-packet}
+@tab @code{vFile:stat}
+@tab Host I/O
+
 @item @code{hostio-setfs-packet}
 @tab @code{vFile:setfs}
 @tab Host I/O
@@ -46326,6 +46330,13 @@
 If an error occurs the return value is -1.  The format of the
 returned binary attachment is as described in @ref{struct stat}.
 
+@item vFile:stat: @var{filename}
+Get information about the file @var{filename} on the target.
+On success the information is returned as a binary attachment
+and the return value is the size of this attachment in bytes.
+If an error occurs the return value is -1.  The format of the
+returned binary attachment is as described in @ref{struct stat}.
+
 @item vFile:unlink: @var{filename}
 Delete the file at @var{filename} on the target.  Return 0,
 or -1 if an error occurs.  The @var{filename} is a string.
diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 1318d6b041e..df993b624dd 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -320,6 +320,21 @@ inf_child_target::fileio_fstat (int fd, struct stat *sb, fileio_error *target_er
   return ret;
 }
 
+/* Implementation of to_fileio_stat.  */
+
+int
+inf_child_target::fileio_stat (struct inferior *inf, const char *filename,
+			       struct stat *sb, fileio_error *target_errno)
+{
+  int ret;
+
+  ret = lstat (filename, sb);
+  if (ret == -1)
+    *target_errno = host_to_fileio_error (errno);
+
+  return ret;
+}
+
 /* Implementation of to_fileio_close.  */
 
 int
diff --git a/gdb/inf-child.h b/gdb/inf-child.h
index 91955a64f4c..65d42e135ce 100644
--- a/gdb/inf-child.h
+++ b/gdb/inf-child.h
@@ -81,6 +81,8 @@ class inf_child_target
   int fileio_pread (int fd, gdb_byte *read_buf, int len,
 		    ULONGEST offset, fileio_error *target_errno) override;
   int fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno) override;
+  int fileio_stat (struct inferior *inf, const char *filename,
+		   struct stat *sb, fileio_error *target_errno) override;
   int fileio_close (int fd, fileio_error *target_errno) override;
   int fileio_unlink (struct inferior *inf,
 		     const char *filename,
diff --git a/gdb/remote.c b/gdb/remote.c
index 90a4bd57a82..8b1f8d7f7db 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -248,6 +248,7 @@ enum {
   PACKET_vFile_unlink,
   PACKET_vFile_readlink,
   PACKET_vFile_fstat,
+  PACKET_vFile_stat,
   PACKET_qXfer_auxv,
   PACKET_qXfer_features,
   PACKET_qXfer_exec_file,
@@ -1010,6 +1011,9 @@ class remote_target : public process_stratum_target
 
   int fileio_fstat (int fd, struct stat *sb, fileio_error *target_errno) override;
 
+  int fileio_stat (struct inferior *inf, const char *filename,
+		   struct stat *sb, fileio_error *target_errno) override;
+
   int fileio_close (int fd, fileio_error *target_errno) override;
 
   int fileio_unlink (struct inferior *inf,
@@ -13048,6 +13052,41 @@ remote_target::fileio_readlink (struct inferior *inf, const char *filename,
   return ret;
 }
 
+/* Helper function to handle ::fileio_fstat and ::fileio_stat result
+   processing.  When this function is called the remote syscall has been
+   performed and we know we didn't get an error back.
+
+   ATTACHMENT and ATTACHMENT_LEN are the attachment data extracted from the
+   remote syscall reply.  EXPECTED_LEN is the length returned from the
+   fstat or stat call, this the length of the returned data (in ATTACHMENT)
+   once it has been decoded.  The fstat/stat result (from the ATTACHMENT
+   data) is to be placed in ST.  */
+
+static int
+fileio_process_fstat_and_stat_reply (const char *attachment,
+				     int attachment_len,
+				     int expected_len,
+				     struct stat *st)
+{
+  struct fio_stat fst;
+
+  int read_len
+    = remote_unescape_input ((gdb_byte *) attachment, attachment_len,
+			     (gdb_byte *) &fst, sizeof (fst));
+
+  if (read_len != expected_len)
+    error (_("vFile:fstat returned %d, but %d bytes."),
+	   expected_len, read_len);
+
+  if (read_len != sizeof (fst))
+    error (_("vFile:fstat returned %d bytes, but expecting %d."),
+	   read_len, (int) sizeof (fst));
+
+  remote_fileio_to_host_stat (&fst, st);
+
+  return 0;
+}
+
 /* Implementation of to_fileio_fstat.  */
 
 int
@@ -13058,8 +13097,6 @@ remote_target::fileio_fstat (int fd, struct stat *st, fileio_error *remote_errno
   int left = get_remote_packet_size ();
   int attachment_len, ret;
   const char *attachment;
-  struct fio_stat fst;
-  int read_len;
 
   remote_buffer_add_string (&p, &left, "vFile:fstat:");
 
@@ -13091,19 +13128,41 @@ remote_target::fileio_fstat (int fd, struct stat *st, fileio_error *remote_errno
       return 0;
     }
 
-  read_len = remote_unescape_input ((gdb_byte *) attachment, attachment_len,
-				    (gdb_byte *) &fst, sizeof (fst));
+  return fileio_process_fstat_and_stat_reply (attachment, attachment_len,
+					      ret, st);
+}
 
-  if (read_len != ret)
-    error (_("vFile:fstat returned %d, but %d bytes."), ret, read_len);
+/* Implementation of to_fileio_stat.  */
 
-  if (read_len != sizeof (fst))
-    error (_("vFile:fstat returned %d bytes, but expecting %d."),
-	   read_len, (int) sizeof (fst));
+int
+remote_target::fileio_stat (struct inferior *inf, const char *filename,
+			    struct stat *st, fileio_error *remote_errno)
+{
+  struct remote_state *rs = get_remote_state ();
+  char *p = rs->buf.data ();
+  int left = get_remote_packet_size () - 1;
 
-  remote_fileio_to_host_stat (&fst, st);
+  if (remote_hostio_set_filesystem (inf, remote_errno) != 0)
+    return {};
 
-  return 0;
+  remote_buffer_add_string (&p, &left, "vFile:stat:");
+
+  remote_buffer_add_bytes (&p, &left, (const gdb_byte *) filename,
+			   strlen (filename));
+
+  int attachment_len;
+  const char *attachment;
+  int ret = remote_hostio_send_command (p - rs->buf.data (), PACKET_vFile_stat,
+					remote_errno, &attachment,
+					&attachment_len);
+
+  /* Unlike ::fileio_fstat, the stat fileio call was added later on, and
+     has none of the legacy bfd issues, so we can just return the error.  */
+  if (ret < 0)
+    return ret;
+
+  return fileio_process_fstat_and_stat_reply (attachment, attachment_len,
+					      ret, st);
 }
 
 /* Implementation of to_filesystem_is_local.  */
@@ -16178,6 +16237,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
 
   add_packet_config_cmd (PACKET_vFile_fstat, "vFile:fstat", "hostio-fstat", 0);
 
+  add_packet_config_cmd (PACKET_vFile_stat, "vFile:stat", "hostio-stat", 0);
+
   add_packet_config_cmd (PACKET_vAttach, "vAttach", "attach", 0);
 
   add_packet_config_cmd (PACKET_vRun, "vRun", "run", 0);
-- 
2.25.4

[PATCHv2 3/4] gdbserver: add gdbserver support for vFile::stat packet

[Andrew Burgess]

After the previous two commits, this commit adds support for the
vFile::stat packet to gdbserver.  This is pretty similar to the
handling for vFile::fstat, but instead calls 'lstat'.

There's still no users of target_fileio_stat in GDB, that will come in
a later commit.
---
 gdbserver/hostio.cc | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gdbserver/hostio.cc b/gdbserver/hostio.cc
index c5ae35ab7dd..cc47d6869d3 100644
--- a/gdbserver/hostio.cc
+++ b/gdbserver/hostio.cc
@@ -486,6 +486,42 @@ handle_fstat (char *own_buf, int *new_packet_len)
     write_enn (own_buf);
 }
 
+static void
+handle_stat (char *own_buf, int *new_packet_len)
+{
+  int bytes_sent;
+  char *p;
+  struct stat st;
+  struct fio_stat fst;
+  char filename[HOSTIO_PATH_MAX];
+
+  p = own_buf + strlen ("vFile:stat:");
+
+  if (require_filename (&p, filename)
+      || require_end (p))
+    {
+      hostio_packet_error (own_buf);
+      return;
+    }
+
+  if (lstat (filename, &st) == -1)
+    {
+      hostio_error (own_buf);
+      return;
+    }
+
+  host_to_fileio_stat (&st, &fst);
+
+  bytes_sent = hostio_reply_with_data (own_buf,
+				       (char *) &fst, sizeof (fst),
+				       new_packet_len);
+
+  /* If the response does not fit into a single packet, do not attempt
+     to return a partial response, but simply fail.  */
+  if (bytes_sent < sizeof (fst))
+    write_enn (own_buf);
+}
+
 static void
 handle_close (char *own_buf)
 {
@@ -603,6 +639,8 @@ handle_vFile (char *own_buf, int packet_len, int *new_packet_len)
     handle_pwrite (own_buf, packet_len);
   else if (startswith (own_buf, "vFile:fstat:"))
     handle_fstat (own_buf, new_packet_len);
+  else if (startswith (own_buf, "vFile:stat:"))
+    handle_stat (own_buf, new_packet_len);
   else if (startswith (own_buf, "vFile:close:"))
     handle_close (own_buf);
   else if (startswith (own_buf, "vFile:unlink:"))
-- 
2.25.4

[PATCHv2 4/4] gdb: check for multiple matching build-id files

[Andrew Burgess]

Within the debug-file-directory GDB looks for the existence of a
.build-id directory.

Within the .build-id directory GDB looks for files with the form:

  .build-id/ff/4b4142d62b399499844924d53e33d4028380db.debug

which contain the debug information for the objfile with the build-id
ff4b4142d62b399499844924d53e33d4028380db.

There appear to be two strategies for populating the .build-id
directory.  Ubuntu takes the approach of placing the actual debug
information in this directory, so
4b4142d62b399499844924d53e33d4028380db.debug is an actual file
containing the debug information.

Fedora, RHEL, and SUSE take a slightly different approach, placing the
debug information elsewhere, and then creating symlinks in the
.build-id directory back to the original debug information file.  The
actual debug information is arranged in a mirror of the filesystem
within the debug directory, as an example, if the debug-file-directory
is /usr/lib/debug, then the debug information for /bin/foo can be
found in /usr/lib/debug/bin/foo.debug.

Where this gets interesting is that in some cases a package will
install a single binary with multiple names, in this case a single
binary will be install with either hard-links, or symlinks providing
the alternative names.

The debug information for these multiple binaries will then be placed
into the /usr/lib/debug/ tree, and again, links are created so a
single file can provide debug information for each of the names that
binary presents as.

In the .build-id tree though we have a problem.  Do we have a single
entry that links to one of the binary names?  This would work; a user
debugging any of the binaries will find the debug information based on
the build-id, and will get the correct information, after all the
binaries are identical (same file linked together).  But there is one
problem with this approach.

Sometimes, for *reasons* it's possible that one or more the linked
binaries might get removed.  I'm honestly not 100% certain under what
circumstances this can happen, but what I observe is that sometime a
single name for a binary, and its corresponding .debug entry, can be
removed.  If this happens to be the entry that the .build-id link is
pointing at, then we have a problem.  The user can no longer find the
debug information based on the .build-id.

The solution that Fedora, RHEL, & SUSE have adopted is to add multiple
entries in the .build-id tree, with each entry pointing to a different
name within the debug/ tree, a sequence number is added to the
build-id to distinguish the multiple entries.  Thus, we might end up
with a layout like this:

  /bin/
    binary_name_1
    binary_name_2
    binary_name_3
  /usr/
    lib/
      debug/
        bin/
	  binary_name_1.debug
	  binary_name_2.debug -> binary_name_1.debug	[ HARDLINK ]
	  binary_name_3.debug -> binary_name_1.debug    [ HARDLINK ]
        .build-id/
	  a3/
	    4b4142d62b399499844924d53e33d4028380db.debug -> ../../../debug/bin/binary_name_1.debug	[ SYMLINK ]
	    4b4142d62b399499844924d53e33d4028380db.1.debug -> ../../../debug/bin/binary_name_2.debug	[ SYMLINK ]
	    4b4142d62b399499844924d53e33d4028380db.2.debug -> ../../../debug/bin/binary_name_3.debug	[ SYMLINK ]

With current master GDB, debug information will only ever be looked up
via the 4b4142d62b399499844924d53e33d4028380db.debug link.  But if
binary_name_1 and its corresponding binary_name_1.debug are ever
removed, then master GDB will fail to find the debug information.

Ubuntu seems to have a much better approach for debug information
handling; they place the debug information directly into the .build-id
tree, so there only ever needs to be a single entry for any one
build-id.  It's not clear if/how they handle the case where multiple
names might share a single .debug file, if one of those names is then
uninstalled, how do they know the .debug file should be retained or
not ... but I assume that problem either doesn't exist or has been
solved.

Anyway, for a while Fedora has carried a patch that handles the
build-id sequence number logic, and I'd like to try and get this patch
upstream.

I'm aware that this is a patch that applies to only some (probably a
minority) of distros.  However, the logic is contained to only a
single function in build-id.c, and isn't that complex, so I'm hoping
that there wont be too many objections.

For distros that don't have build-id sequence numbers then they should
see no change.  The unnumbered file is checked for first, and only if
that exists, but isn't readable, do we check for numbered files
instead.

Tests are included.

There is a small fix needed for gdb.base/sysroot-debug-lookup.exp,
after this commit GDB now treats a target: sysroot where the target
file system is local to GDB the same as if the sysroot had no target:
prefix.  The consequence of this is that GDB now resolves a symlink
back to the real filename in the sysroot-debug-lookup.exp test where
it didn't previously.  As this behaviour is inline with the case where
there is no target: prefix I think this is fine.
---
 gdb/build-id.c                                | 175 ++++++++++++----
 gdb/testsuite/gdb.base/build-id-seqno.c       |  22 ++
 gdb/testsuite/gdb.base/build-id-seqno.exp     | 133 ++++++++++++
 .../gdb.base/sysroot-debug-lookup.exp         |  13 +-
 gdb/testsuite/gdb.server/build-id-seqno.c     |  22 ++
 gdb/testsuite/gdb.server/build-id-seqno.exp   | 198 ++++++++++++++++++
 6 files changed, 517 insertions(+), 46 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.base/build-id-seqno.exp
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.c
 create mode 100644 gdb/testsuite/gdb.server/build-id-seqno.exp

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 8ce94f1e7a0..1469222ca49 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -73,50 +73,150 @@ build_id_verify (bfd *abfd, size_t check_len, const bfd_byte *check)
   return retval;
 }
 
-/* Helper for build_id_to_debug_bfd.  LINK is a path to a potential
-   build-id-based separate debug file, potentially a symlink to the real file.
-   If the file exists and matches BUILD_ID, return a BFD reference to it.  */
+/* Helper for build_id_to_debug_bfd.  ORIGINAL_LINK with SUFFIX appended is
+   a path to a potential build-id-based separate debug file, potentially a
+   symlink to the real file.  If the file exists and matches BUILD_ID,
+   return a BFD reference to it.  */
 
 static gdb_bfd_ref_ptr
-build_id_to_debug_bfd_1 (const std::string &link, size_t build_id_len,
-			 const bfd_byte *build_id)
+build_id_to_debug_bfd_1 (const std::string &original_link,
+			 size_t build_id_len, const bfd_byte *build_id,
+			 const char *suffix)
 {
-  separate_debug_file_debug_printf ("Trying %s...", link.c_str ());
-
-  /* lrealpath() is expensive even for the usually non-existent files.  */
-  gdb::unique_xmalloc_ptr<char> filename_holder;
-  const char *filename = nullptr;
-  if (is_target_filename (link))
-    filename = link.c_str ();
-  else if (access (link.c_str (), F_OK) == 0)
+  tribool supports_target_stat = TRIBOOL_UNKNOWN;
+
+  /* Drop the 'target:' prefix if the target filesystem is local.  */
+  std::string_view original_link_view (original_link);
+  if (is_target_filename (original_link) && target_filesystem_is_local ())
+    original_link_view
+      = original_link_view.substr (strlen (TARGET_SYSROOT_PREFIX));
+
+  /* The upper bound of '10' here is completely arbitrary.  The loop should
+     terminate via 'break' when either (a) a readable symlink is found, or
+     (b) a non-existing entry is found.
+
+     However, for remote targets, we rely on the remote returning sane
+     error codes.  If a remote sends back the wrong error code then it
+     might trick GDB into thinking that the symlink exists, but points to a
+     missing file, in which case GDB will try the next seqno.  We don't
+     want a broken remote to cause GDB to spin here forever, hence a fixed
+     upper bound.  */
+
+  for (unsigned seqno = 0; seqno < 10; seqno++)
     {
-      filename_holder.reset (lrealpath (link.c_str ()));
-      filename = filename_holder.get ();
-    }
+      std::string link (original_link_view);
 
-  if (filename == NULL)
-    {
-      separate_debug_file_debug_printf ("unable to compute real path");
-      return {};
-    }
+      if (seqno > 0)
+	string_appendf (link, ".%u", seqno);
 
-  /* We expect to be silent on the non-existing files.  */
-  gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename, gnutarget);
+      link += suffix;
 
-  if (debug_bfd == NULL)
-    {
-      separate_debug_file_debug_printf ("unable to open.");
-      return {};
-    }
+      separate_debug_file_debug_printf ("Trying %s...", link.c_str ());
 
-  if (!build_id_verify (debug_bfd.get(), build_id_len, build_id))
-    {
-      separate_debug_file_debug_printf ("build-id does not match.");
-      return {};
+      gdb::unique_xmalloc_ptr<char> filename_holder;
+      const char *filename = nullptr;
+      if (is_target_filename (link))
+	{
+	  gdb_assert (link.length () >= strlen (TARGET_SYSROOT_PREFIX));
+	  const char *link_on_target
+	    = link.c_str () + strlen (TARGET_SYSROOT_PREFIX);
+
+	  fileio_error target_errno;
+	  if (supports_target_stat != TRIBOOL_FALSE)
+	    {
+	      struct stat sb;
+	      int res = target_fileio_stat (nullptr, link_on_target, &sb,
+					    &target_errno);
+
+	      if (res != 0 && target_errno != FILEIO_ENOSYS)
+		{
+		  supports_target_stat = TRIBOOL_TRUE;
+		  separate_debug_file_debug_printf ("path doesn't exist");
+		  break;
+		}
+	      else if (res != 0 && target_errno == FILEIO_ENOSYS)
+		supports_target_stat = TRIBOOL_FALSE;
+	      else
+		{
+		  supports_target_stat = TRIBOOL_TRUE;
+		  filename = link.c_str ();
+		}
+	    }
+
+	  if (supports_target_stat == TRIBOOL_FALSE)
+	    {
+	      gdb_assert (filename == nullptr);
+
+	      /* Connecting to a target that doesn't support 'stat'.  Try
+		 'readlink' as an alternative.  This isn't ideal, but is
+		 maybe better than nothing.  Returns EINVAL if the path
+		 isn't a symbolic link, which hints that the path is
+		 available -- there are other errors e.g. ENOENT for when
+		 the path doesn't exist, but we just assume that anything
+		 other than EINVAL indicates the path doesn't exist.  */
+	      std::optional<std::string> link_target
+		= target_fileio_readlink (nullptr, link_on_target,
+					  &target_errno);
+	      if (link_target.has_value ()
+		  || target_errno == FILEIO_EINVAL)
+		filename = link.c_str ();
+	      else
+		{
+		  separate_debug_file_debug_printf ("path doesn't exist");
+		  break;
+		}
+	    }
+	}
+      else
+	{
+	  struct stat buf;
+
+	  /* The `access' call below automatically dereferences LINK, but
+	     we want to stop incrementing SEQNO once we find a symlink
+	     that doesn't exist.  */
+	  if (lstat (link.c_str (), &buf) != 0)
+	    {
+	      separate_debug_file_debug_printf ("path doesn't exist");
+	      break;
+	    }
+
+	  /* Can LINK be accessed, or if LINK is a symlink, can the file
+	     pointed too be accessed?  Do this as lrealpath() is
+	     expensive, even for the usually non-existent files.  */
+	  if (access (link.c_str (), F_OK) == 0)
+	    {
+	      filename_holder.reset (lrealpath (link.c_str ()));
+	      filename = filename_holder.get ();
+	    }
+	}
+
+      if (filename == nullptr)
+	{
+	  separate_debug_file_debug_printf ("unable to compute real path");
+	  continue;
+	}
+
+      /* We expect to be silent on the non-existing files.  */
+      gdb_bfd_ref_ptr debug_bfd = gdb_bfd_open (filename, gnutarget);
+
+      if (debug_bfd == NULL)
+	{
+	  separate_debug_file_debug_printf ("unable to open `%s`", filename);
+	  continue;
+	}
+
+      if (!build_id_verify (debug_bfd.get(), build_id_len, build_id))
+	{
+	  separate_debug_file_debug_printf ("build-id does not match");
+	  continue;
+	}
+
+      separate_debug_file_debug_printf ("found a match");
+      return debug_bfd;
     }
 
-  separate_debug_file_debug_printf ("found a match");
-  return debug_bfd;
+  separate_debug_file_debug_printf ("no suitable file found");
+  return {};
 }
 
 /* Common code for finding BFDs of a given build-id.  This function
@@ -156,10 +256,8 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
       while (size-- > 0)
 	string_appendf (link, "%02x", (unsigned) *data++);
 
-      link += suffix;
-
       gdb_bfd_ref_ptr debug_bfd
-	= build_id_to_debug_bfd_1 (link, build_id_len, build_id);
+	= build_id_to_debug_bfd_1 (link, build_id_len, build_id, suffix);
       if (debug_bfd != NULL)
 	return debug_bfd;
 
@@ -176,7 +274,8 @@ build_id_to_bfd_suffix (size_t build_id_len, const bfd_byte *build_id,
 	      || !target_filesystem_is_local ()))
 	{
 	  link = gdb_sysroot + link;
-	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id);
+	  debug_bfd = build_id_to_debug_bfd_1 (link, build_id_len, build_id,
+					       suffix);
 	  if (debug_bfd != NULL)
 	    return debug_bfd;
 	}
diff --git a/gdb/testsuite/gdb.base/build-id-seqno.c b/gdb/testsuite/gdb.base/build-id-seqno.c
new file mode 100644
index 00000000000..e2119ba7d92
--- /dev/null
+++ b/gdb/testsuite/gdb.base/build-id-seqno.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/build-id-seqno.exp b/gdb/testsuite/gdb.base/build-id-seqno.exp
new file mode 100644
index 00000000000..b13ac8edd86
--- /dev/null
+++ b/gdb/testsuite/gdb.base/build-id-seqno.exp
@@ -0,0 +1,133 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2024 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/>.
+
+# Setup a .build-id/ based debug directory containing multiple entries
+# for the same build-id, with each entry given a different sequence
+# number.
+#
+# Ensure that GDB will scan over broken symlinks for the same build-id
+# (but different sequence number) to find later working symlinks.
+#
+# This test only places debug information on the host, so it is always
+# local to GDB.
+
+require {!is_remote host}
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile] == -1} {
+    return -1
+}
+
+# Split out BINFILE.debug.  Remove debug from BINFILE.
+if {[gdb_gnu_strip_debug $binfile] != 0} {
+    return -1
+}
+
+# Get the '.build-id/xx/xxx...xxx' part of the filename.
+set build_id_filename [build_id_debug_filename_get $binfile]
+
+# Hide (rename) BINFILE.debug, this should ensure GDB can't find it
+# directly but needs to look for the build-id based file in the debug
+# directory.
+set hidden_debuginfo [standard_output_file "hidden_$testfile.debug"]
+remote_exec build "mv ${binfile}.debug $hidden_debuginfo"
+
+# A filename that doesn't exist.  Some symlinks will point at this
+# file.
+set missing_debuginfo [host_standard_output_file "missing_debuginfo"]
+
+# Create the debug directory, and the .build-id directory structure
+# within it.
+set debugdir [host_standard_output_file "debug"]
+remote_exec host "mkdir -p $debugdir/[file dirname $build_id_filename]"
+
+set host_hidden_debuginfo [gdb_remote_download host $hidden_debuginfo]
+remote_exec host "ln -fs $host_hidden_debuginfo $debugdir/$build_id_filename"
+
+# Start GDB and load global BINFILE.  If FIND_DEBUGINFO is true then
+# we expect GDB to find the debug information matching BINFILE,
+# otherwise, we expect GDB not to find the debug information.
+proc load_binfile_check_debug_is_found { find_debuginfo testname } {
+    with_test_prefix "$testname" {
+	clean_restart
+
+	gdb_test_no_output "set debug-file-directory $::debugdir" \
+	    "set debug-file-directory"
+
+	gdb_file_cmd $::binfile
+
+	if { $find_debuginfo } {
+	    gdb_assert { [regexp [string_to_regexp \
+				      "Reading symbols from $::hidden_debuginfo..."] \
+			      $::gdb_file_cmd_msg] } \
+		"debuginfo was read via build-id"
+	} else {
+	    gdb_assert { [regexp "\\(No debugging symbols found in \[^\r\n\]+/$::testfile\\)" \
+			      $::gdb_file_cmd_msg] } \
+	    }
+    }
+}
+
+# Return a copy of FILENAME, which should end '.debug', with NUMBER
+# added, e.g. add_seqno 1 "foo.debug" --> "foo.1.debug".
+proc add_seqno { number filename } {
+    return [regsub "\.debug\$" $filename ".${number}.debug"]
+}
+
+load_binfile_check_debug_is_found true \
+    "find debuginfo with a single build-id file"
+
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 1 $build_id_filename]"
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 2 $build_id_filename]"
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 3 $build_id_filename]"
+
+load_binfile_check_debug_is_found true \
+    "find debuginfo with 4 build-id files"
+
+remote_exec host "ln -fs $missing_debuginfo $debugdir/$build_id_filename"
+
+load_binfile_check_debug_is_found true \
+    "find debuginfo, first build-id file is bad"
+
+remote_exec host "ln -fs $missing_debuginfo \
+	    	     $debugdir/[add_seqno 1 $build_id_filename]"
+remote_exec host "ln -fs $missing_debuginfo \
+	    	     $debugdir/[add_seqno 3 $build_id_filename]"
+
+load_binfile_check_debug_is_found true  \
+    "find debuginfo, first 2 build-id files are bad"
+
+remote_exec host "ln -fs $missing_debuginfo \
+	    	     $debugdir/[add_seqno 2 $build_id_filename]"
+
+load_binfile_check_debug_is_found false  \
+    "cannot find debuginfo, all build-id files are bad"
+
+remote_exec host "ln -fs $host_hidden_debuginfo \
+	    	     $debugdir/[add_seqno 3 $build_id_filename]"
+
+load_binfile_check_debug_is_found true  \
+    "find debuginfo, last build-id file is good"
+
+remote_exec host "rm -f $debugdir/[add_seqno 1 $build_id_filename]"
+
+load_binfile_check_debug_is_found false  \
+    "cannot find debuginfo, file with seqno 1 is missing"
diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
index 5f17315c027..36f65192ca3 100644
--- a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
+++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
@@ -78,9 +78,8 @@ proc_with_prefix lookup_via_build_id {} {
 	gdb_assert { $::gdb_file_cmd_debug_info eq "debug" } \
 	    "ensure debug information was found"
 
-	if { $sysroot_prefix eq "" } {
-	    set lookup_filename $debug_filename
-	} else {
+	if { $sysroot_prefix eq "target:"
+	     && [target_info gdb_protocol] == "extended-remote"} {
 	    # Only when using the extended-remote board will we have
 	    # started a remote target by this point.  In this case GDB
 	    # will see the 'target:' prefix as remote, and so the
@@ -89,11 +88,9 @@ proc_with_prefix lookup_via_build_id {} {
 	    # In all other cases we will still be using the default,
 	    # initial target, in which case GDB considers the
 	    # 'target:' prefix to indicate the local filesystem.
-	    if {[target_info gdb_protocol] == "extended-remote"} {
-		set lookup_filename $sysroot_prefix$debug_symlink
-	    } else {
-		set lookup_filename $debug_symlink
-	    }
+	    set lookup_filename $sysroot_prefix$debug_symlink
+	} else {
+	    set lookup_filename $debug_filename
 	}
 	set re [string_to_regexp "Reading symbols from $lookup_filename..."]
 	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
diff --git a/gdb/testsuite/gdb.server/build-id-seqno.c b/gdb/testsuite/gdb.server/build-id-seqno.c
new file mode 100644
index 00000000000..e2119ba7d92
--- /dev/null
+++ b/gdb/testsuite/gdb.server/build-id-seqno.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.server/build-id-seqno.exp b/gdb/testsuite/gdb.server/build-id-seqno.exp
new file mode 100644
index 00000000000..c565d5a4348
--- /dev/null
+++ b/gdb/testsuite/gdb.server/build-id-seqno.exp
@@ -0,0 +1,198 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2024 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/>.
+
+# Setup a .build-id/ based debug directory containing multiple entries
+# for the same build-id, with each entry given a different sequence
+# number.
+#
+# Ensure that GDB will scan over broken symlinks for the same build-id
+# (but different sequence number) to find later working symlinks.
+#
+# This test places the build-id files within a directory next to where
+# gdbserver is started, and places a relative address in the
+# debug-file-directory, in this way we require GDB to find the debug
+# information via gdbserver.
+
+require {!is_remote host}
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+if {[build_executable "failed to prepare" $testfile $srcfile] == -1} {
+    return -1
+}
+
+# Split out BINFILE.debug.  Remove debug from BINFILE.
+if {[gdb_gnu_strip_debug $binfile] != 0} {
+    return -1
+}
+
+# Get the '.build-id/xx/xxx...xxx' part of the filename.
+set build_id_filename [build_id_debug_filename_get $binfile]
+
+# Hide (rename) BINFILE.debug, this should ensure GDB can't find it
+# directly but needs to look for the build-id based file in the debug
+# directory.
+set hidden_debuginfo [standard_output_file "hidden_$testfile.debug"]
+remote_exec build "mv ${binfile}.debug $hidden_debuginfo"
+
+# A filename that doesn't exist.  Some symlinks will point at this
+# file.
+set missing_debuginfo "missing_debuginfo"
+
+# Helper called from gdb_finish when the 'target' is remote.  Ensure the
+# debug directory we create is deleted.
+proc cleanup_remote_target {} {
+    remote_exec target "rm -fr debug/"
+}
+
+if { ![is_remote target] } {
+    set gdbserver_dir [standard_output_file "gdbserver-dir"]/
+} else {
+    lappend gdb_finish_hooks cleanup_remote_target
+    set gdbserver_dir ""
+}
+
+# Copy files to the target (if needed).
+set target_binfile [gdb_remote_download target $binfile]
+set target_debuginfo [gdb_remote_download target $hidden_debuginfo]
+
+# Setup the debug information on the target.
+set debugdir "${gdbserver_dir}debug"
+remote_exec target \
+    "mkdir -p $debugdir/[file dirname $build_id_filename]"
+remote_exec target \
+    "ln -sf $target_debuginfo $debugdir/$build_id_filename"
+
+# Start GDB and load global BINFILE.  If DEBUGINFO_FILE is not the
+# empty string then this contains the '.build-id/xx/xxx....xxxx' part
+# of the filename which we expect GDB to read from the remote target.
+# If DEBUGINFO_FILE is the empty string then we don't expect GDB to
+# find any debug information.
+proc load_binfile_check_debug_is_found { debuginfo_file testname } {
+    with_test_prefix "$testname" {
+	with_timeout_factor 5 {
+	    # Probing for .build-id based debug files on remote
+	    # targets uses the vFile:stat packet by default, though
+	    # there is a work around that avoids this which can be
+	    # used if GDB is connected to an older gdbserver without
+	    # 'stat' support.
+	    #
+	    # Check the work around works by disabling use of the
+	    # vFile:stat packet.
+	    foreach_with_prefix stat_pkt {auto off} {
+		clean_restart
+
+		gdb_test_no_output "set debug-file-directory debug" \
+		    "set debug-file-directory"
+
+		gdb_test_no_output "set sysroot target:"
+
+		gdb_test "set remote hostio-stat-packet $stat_pkt"
+
+		# Make sure we're disconnected, in case we're testing with an
+		# extended-remote board, therefore already connected.
+		gdb_test "disconnect" ".*"
+
+		# Start gdbserver.  This needs to be done after starting GDB.  When
+		# gdbserver is running local to GDB, start gdbserver in a sub-directory,
+		# this prevents GDB from finding the debug information itself.
+		if { ![is_remote target] } {
+		    with_cwd $::gdbserver_dir {
+			set res [gdbserver_start "" $::target_binfile]
+		    }
+		} else {
+		    set res [gdbserver_start "" $::target_binfile]
+		}
+		set gdbserver_protocol [lindex $res 0]
+		set gdbserver_gdbport [lindex $res 1]
+
+		# Connect to gdbserver.  The output will be placed into the global
+		# GDB_TARGET_REMOTE_CMD_MSG, and we'll match against this below.
+		gdb_assert {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport] == 0} \
+		    "connect to gdbserver"
+
+		if { $debuginfo_file ne "" } {
+		    gdb_assert { [regexp "Reading symbols from target:debug/[string_to_regexp $debuginfo_file]\\.\\.\\." \
+				      $::gdb_target_remote_cmd_msg] } \
+			"debuginfo was read via build-id"
+		    gdb_assert { [regexp "Reading debug/[string_to_regexp $debuginfo_file] from remote target\\.\\.\\." \
+				      $::gdb_target_remote_cmd_msg] } \
+			"debuginfo was read from remote target"
+		} else {
+		    gdb_assert { [regexp "\\(No debugging symbols found in \[^\r\n\]+/$::testfile\\)" \
+				      $::gdb_target_remote_cmd_msg] }
+		}
+	    }
+	}
+    }
+}
+
+# Return a copy of FILENAME, which should end '.debug', with NUMBER
+# added, e.g. add_seqno 1 "foo.debug" --> "foo.1.debug".
+proc add_seqno { number filename } {
+    return [regsub "\.debug\$" $filename ".${number}.debug"]
+}
+
+# Precompute sequence numbered build-id filenames.
+set build_id_1_filename [add_seqno 1 $build_id_filename]
+set build_id_2_filename [add_seqno 2 $build_id_filename]
+set build_id_3_filename [add_seqno 3 $build_id_filename]
+
+load_binfile_check_debug_is_found $build_id_filename \
+    "find debuginfo with a single build-id file"
+
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_1_filename"
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_2_filename"
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_3_filename"
+
+load_binfile_check_debug_is_found $build_id_filename \
+    "find debuginfo with 4 build-id files"
+
+remote_exec target "ln -fs $missing_debuginfo $debugdir/$build_id_filename"
+
+load_binfile_check_debug_is_found $build_id_1_filename \
+    "find debuginfo, first build-id file is bad"
+
+remote_exec target "ln -fs $missing_debuginfo \
+	    	     $debugdir/$build_id_1_filename"
+remote_exec target "ln -fs $missing_debuginfo \
+	    	     $debugdir/$build_id_3_filename"
+
+load_binfile_check_debug_is_found $build_id_2_filename  \
+    "find debuginfo, first 2 build-id files are bad"
+
+remote_exec target "ln -fs $missing_debuginfo \
+	    	     $debugdir/$build_id_2_filename"
+
+load_binfile_check_debug_is_found ""  \
+    "cannot find debuginfo, all build-id files are bad"
+
+remote_exec target "ln -fs $target_debuginfo \
+	    	     $debugdir/$build_id_3_filename"
+
+load_binfile_check_debug_is_found $build_id_3_filename  \
+    "find debuginfo, last build-id file is good"
+
+remote_exec target "rm -f $debugdir/$build_id_1_filename"
+
+load_binfile_check_debug_is_found ""  \
+    "cannot find debuginfo, file with seqno 1 is missing"
-- 
2.25.4