gdb: avoid '//' in filenames when searching for debuginfo

I spotted that the gdb.base/sysroot-debug-lookup.exp test that I added
recently actually had a KPASS when run with the
native-extended-gdbserver board.  This was an oversight when adding
the test.

The failures in this test, when using the 'unix' board, are logged as
bug PR gdb/31804.  The problem appears to be caused by the use of the
child_path function in find_separate_debug_file.

What happens on the 'unix' board is that the file is specified to GDB
with a target: prefix, however GDB spots that the target filesystem is
local to GDB and so opens the file without a target: prefix.  When we
call into find_separate_debug_file the DIR and CANON_DIR arguments,
which are computed from the objfile_name() no longer have a target:
prefix.

However, in this test if the file was opened with a target: prefix,
then the sysroot also has a target: prefix.  When child_path is called
it looks for a common prefix between CANON_DIR (from the objfile_name)
and the sysroot.  However, the sysroot still has the target: prefix,
which means the child_path() call fails and returns nullptr.

What happens in the native-extended-gdbserver case is that GDB doesn't
see the target filesystem as local.  Now the filename retains the
target: prefix, which means that in the child_path() call both the
sysroot and the CANON_DIR have a target: prefix, and so the
child_path() call succeeds.  This allows GDB to progress, try some
additional paths, and then find the debug information.

So, this commit changes gdb.base/sysroot-debug-lookup.exp to expect
the test to succeed when using the native-extended-gdbserver protocol.

This leaves one KFAIL when using the native-extended-gdbserver board,
we find the debug information but (apparently) find it in the wrong
file.  What's happening is that when GDB builds the filename for the
debug information we end up with a '//' string as a directory
separator, the test regexp only expects a single separator.

Instead of just fixing the test regexp, I've updated the path_join
function in gdbsupport/pathstuff.{cc,h} to allow for absolute paths to
appear in the argument list after the first argument.  This means it's
now possible to do this:

  auto result = path_join ("/a/b/c", "/d/e/f");
  gdb_assert (result == "/a/b/c/d/e/f");

Additionally I've changed path_join so that it avoids adding
unnecessary directory separators.  In the above case when the two
paths were joined GDB only added a single separator between 'c' and
'd'.  But additionally, if we did this:

  auto result = path_join ("/a/b/c/", "/d/e/f");
  gdb_assert (result == "/a/b/c/d/e/f");

We'd still only get a single separator.

With these changes to path_join I can now make use of this function in
find_separate_debug_file.  With this done I now have no KFAIL when
using the native-extended-gdbserver board.

After this commit we still have 2 KFAIL when not using the
native-gdbserver and unix boards, these will be addressed in the next
commit.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31804

Reviewed-By: Keith Seitz <keiths@redhat.com>

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 49b98459a28a5caffa4661701f674da79d0d1c99..11313c858c6cf69870df7e41b28507f0c24ae561 100644 (file)
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1371,17 +1371,13 @@ find_separate_debug_file (const char *dir,
      objfile_name (objfile));
 
   /* First try in the same directory as the original file.  */
-  std::string debugfile = dir;
-  debugfile += debuglink;
+  std::string debugfile = path_join (dir, debuglink);
 
   if (separate_debug_file_exists (debugfile, crc32, objfile, warnings))
     return debugfile;
 
   /* Then try in the subdirectory named DEBUG_SUBDIRECTORY.  */
-  debugfile = dir;
-  debugfile += DEBUG_SUBDIRECTORY;
-  debugfile += "/";
-  debugfile += debuglink;
+  debugfile = path_join (dir, DEBUG_SUBDIRECTORY, debuglink);
 
   if (separate_debug_file_exists (debugfile, crc32, objfile, warnings))
     return debugfile;
@@ -1394,6 +1390,7 @@ find_separate_debug_file (const char *dir,
   bool target_prefix = is_target_filename (dir);
   const char *dir_notarget
     = target_prefix ? dir + strlen (TARGET_SYSROOT_PREFIX) : dir;
+  const char *target_prefix_str = target_prefix ? TARGET_SYSROOT_PREFIX : "";
   std::vector<gdb::unique_xmalloc_ptr<char>> debugdir_vec
     = dirnames_to_char_ptr_vec (debug_file_directory.c_str ());
   gdb::unique_xmalloc_ptr<char> canon_sysroot
@@ -1422,12 +1419,8 @@ find_separate_debug_file (const char *dir,
 
   for (const gdb::unique_xmalloc_ptr<char> &debugdir : debugdir_vec)
     {
-      debugfile = target_prefix ? TARGET_SYSROOT_PREFIX : "";
-      debugfile += debugdir;
-      debugfile += "/";
-      debugfile += drive;
-      debugfile += dir_notarget;
-      debugfile += debuglink;
+      debugfile = path_join (target_prefix_str, debugdir.get (),
+                            drive.c_str (), dir_notarget, debuglink);
 
       if (separate_debug_file_exists (debugfile, crc32, objfile, warnings))
        return debugfile;
@@ -1444,12 +1437,8 @@ find_separate_debug_file (const char *dir,
        {
          /* If the file is in the sysroot, try using its base path in
             the global debugfile directory.  */
-         debugfile = target_prefix ? TARGET_SYSROOT_PREFIX : "";
-         debugfile += debugdir;
-         debugfile += "/";
-         debugfile += base_path;
-         debugfile += "/";
-         debugfile += debuglink;
+         debugfile = path_join (target_prefix_str, debugdir.get (),
+                                base_path, debuglink);
 
          if (separate_debug_file_exists (debugfile, crc32, objfile, warnings))
            return debugfile;
@@ -1462,21 +1451,17 @@ find_separate_debug_file (const char *dir,
             same result as above.  */
          if (gdb_sysroot != TARGET_SYSROOT_PREFIX)
            {
-             debugfile = target_prefix ? TARGET_SYSROOT_PREFIX : "";
+             std::string root;
              if (is_target_filename (gdb_sysroot))
                {
-                 std::string root
-                   = gdb_sysroot.substr (strlen (TARGET_SYSROOT_PREFIX));
+                 root = gdb_sysroot.substr (strlen (TARGET_SYSROOT_PREFIX));
                  gdb_assert (!root.empty ());
-                 debugfile += root;
                }
              else
-               debugfile += gdb_sysroot;
-             debugfile += debugdir;
-             debugfile += "/";
-             debugfile += base_path;
-             debugfile += "/";
-             debugfile += debuglink;
+               root = gdb_sysroot;
+
+             debugfile = path_join (target_prefix_str, root.c_str (),
+                                    debugdir.get (), base_path, debuglink);
 
              if (separate_debug_file_exists (debugfile, crc32, objfile,
                                              warnings))

diff --git a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
index 36f65192ca3fe49f1ce85fa8e6369a5ba95e9c62..88be6ff9a7e17581fe90ab4574b4787fffc283e9 100644 (file)
--- a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
+++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
@@ -180,13 +180,15 @@ proc_with_prefix lookup_via_debuglink {} {
        #
        # Bug PR gdb/30866 seems to be the (or a) relevant bug for
        # this problem.
-       if { $sysroot_prefix ne "" } {
+       if { $sysroot_prefix ne ""
+            && [target_info gdb_protocol] ne "extended-remote" } {
            setup_kfail "*-*-*" 31804
        }
        gdb_assert { $::gdb_file_cmd_debug_info eq "debug" } \
            "ensure debug information was found"
 
-       if { $sysroot_prefix ne "" } {
+       if { $sysroot_prefix ne ""
+            && [target_info gdb_protocol] ne "extended-remote"  } {
            setup_kfail "*-*-*" 31804
        }
        set re [string_to_regexp "Reading symbols from ${sysroot_prefix}$debug_symlink..."]

diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
index 9c3816ccc63057d3aadec5e01b36ba382fe54d33..029e3c9a89001688877ea041d414acd41927e349 100644 (file)
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -198,11 +198,17 @@ path_join (gdb::array_view<const char *> paths)
     {
       const char *path = paths[i];
 
-      if (i > 0)
-       gdb_assert (strlen (path) == 0 || !IS_ABSOLUTE_PATH (path));
-
-      if (!ret.empty () && !IS_DIR_SEPARATOR (ret.back ()))
-         ret += '/';
+      if (!ret.empty ())
+       {
+         /* If RET doesn't already end with a separator then add one.  */
+         if (!IS_DIR_SEPARATOR (ret.back ()))
+           ret += '/';
+
+         /* Now that RET ends with a separator, ignore any at the start of
+            PATH.  */
+         while (IS_DIR_SEPARATOR (path[0]))
+           ++path;
+       }
 
       ret.append (path);
     }

diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
index a61ce23efde5c09884396d67337e8126db6be2c8..22170bbabf2016a7d2118c2dd8c74c4d4a176b6a 100644 (file)
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -80,8 +80,10 @@ extern const char *child_path (const char *parent, const char *child);
 
 /* Join elements in PATHS into a single path.
 
-   The first element can be absolute or relative.  All the others must be
-   relative.  */
+   The first element can be absolute or relative.  Only a single directory
+   separator will be placed between elements of PATHS, if one element ends
+   with a directory separator, or an element starts with a directory
+   separator, then these will be collapsed into a single separator.  */
 
 extern std::string path_join (gdb::array_view<const char *> paths);