[PATCHv2,2/2] gdb: fix a target: prefix issue in find_separate_debug_file

Message ID 61bd10bef3e480ee0cfe56b5b2a8e9ba945b9b4e.1721120333.git.aburgess@redhat.com
State New
Headers
Series find debug by debuglink minor fixes |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Andrew Burgess July 16, 2024, 9 a.m. UTC
  Following on from the previous commit, this commit fixes the two KFAIL
in gdb.base/sysroot-debug-lookup.exp when not using the
native-extended-gdbserver board.

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 I think we need to do is this: if the sysroot has a target:
prefix, and the target filesystem is local to GDB, then we should
strip the target: prefix from the sysroot, just as we do when opening
a file (see gdb_bfd_open in gdb_bfd.c).

Now, when we make the child_path() call neither the sysroot nor
CANON_DIR will have a target: prefix, the child_path() call will
succeed, and GDB will correctly find the debug information.

There is just one remaining issue, the last path we look in when
searching for debug information is built by starting with the sysroot,
then adding the debug directory, see this line:

  debugfile = path_join (target_prefix_str, root.c_str (),
                         debugdir.get (), base_path, debuglink);

The target_prefix_str is set to target: if DIR has a target: prefix,
and DIR should have a target: prefix if the file we're looking for was
opened with a target: prefix.  However, in our case the file was in a
local filesystem so GDB stripped the prefix off.

The sysroot however, does have the target: prefix, and the test is
expecting to see GDB look within a file with the target: prefix.

Given that the above line is about looking within a sub-directory of
the sysroot, I think we should not be stripping the target: prefix off
the sysroot path (as we do when building ROOT), instead, we should, in
this case, not use TARGET_PREFIX_STR, and instead just use GDB's
sysroot as it is (in this case with the target: prefix).

With these fixes in place I now see no failures when using the 'unix',
'native-gdbserver', or 'native-extended-gdbserver' boards with this
test, and the KFAILs can be removed.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31804
---
 gdb/symfile.c                                 | 25 ++++++-------------
 .../gdb.base/sysroot-debug-lookup.exp         | 17 -------------
 2 files changed, 7 insertions(+), 35 deletions(-)
  

Patch

diff --git a/gdb/symfile.c b/gdb/symfile.c
index aeee30af50d..fa0ebe2e4ec 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1392,8 +1392,10 @@  find_separate_debug_file (const char *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
-    = gdb_realpath (gdb_sysroot.c_str ());
+  const char *sysroot_str = gdb_sysroot.c_str ();
+  if (is_target_filename (sysroot_str) && target_filesystem_is_local ())
+    sysroot_str += strlen (TARGET_SYSROOT_PREFIX);
+  gdb::unique_xmalloc_ptr<char> canon_sysroot = gdb_realpath (sysroot_str);
 
  /* MS-Windows/MS-DOS don't allow colons in file names; we must
     convert the drive letter into a one-letter directory, so that the
@@ -1443,24 +1445,11 @@  find_separate_debug_file (const char *dir,
 	    return debugfile;
 
 	  /* If the file is in the sysroot, try using its base path in
-	     the sysroot's global debugfile directory.  GDB_SYSROOT
-	     might refer to a target: path; we strip the "target:"
-	     prefix -- but if that would yield the empty string, we
-	     don't bother at all, because that would just give the
-	     same result as above.  */
+	     the sysroot's global debugfile directory.  */
 	  if (gdb_sysroot != TARGET_SYSROOT_PREFIX)
 	    {
-	      std::string root;
-	      if (is_target_filename (gdb_sysroot))
-		{
-		  root = gdb_sysroot.substr (strlen (TARGET_SYSROOT_PREFIX));
-		  gdb_assert (!root.empty ());
-		}
-	      else
-		root = gdb_sysroot;
-
-	      debugfile = path_join (target_prefix_str, root.c_str (),
-				     debugdir.get (), base_path, debuglink);
+	      debugfile = path_join (gdb_sysroot.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 30d492e0d47..7baf7c9c9ec 100644
--- a/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
+++ b/gdb/testsuite/gdb.base/sysroot-debug-lookup.exp
@@ -174,26 +174,9 @@  proc_with_prefix lookup_via_debuglink {} {
 	# in the sysroot.
 	gdb_file_cmd ${sysroot_prefix}$exec_in_sysroot
 
-	# Giving a sysroot a 'target:' prefix doesn't seem to work as
-	# might be expected for debug-link based lookups.  For this
-	# style of lookup GDB only seems to care if the original file
-	# itself had a 'target:' prefix.  The 'target:' prefix on the
-	# sysroot just seems to cause GDB to bail on looking up via
-	# the 'target:' prefixed sysroot.
-	#
-	# Bug PR gdb/30866 seems to be the (or a) relevant bug for
-	# this problem.
-	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 ""
-	     && [target_info gdb_protocol] ne "extended-remote"  } {
-	    setup_kfail "*-*-*" 31804
-	}
 	set re [string_to_regexp "Reading symbols from ${sysroot_prefix}$debug_symlink..."]
 	gdb_assert {[regexp $re $::gdb_file_cmd_msg]} \
 	    "debug symbols read from correct file"