Fix regular /path/to/directory sysroots and target reported dll paths with drive...

I tried debugging a remote Windows program on Linux host, and pointed the
sysroot to "/some/path/" rather than "remote:", and I found GDB couldn't
find the dlls in the sysroot.  If the dll name is
"C:/Windows/system32/ntdll.dll", I end up with the sysroot+in_pathname
concatenated this way:

 (top-gdb) p temp_pathname
 $1 = 0x228b690 "/some/pathC:/Windows/system32/ntdll.dll"
                          ^^

That is, a directory separator is missing.  This is a regression.

The problem is that solib_find decides that since the target path has
a drive spec, a separator is not necessary, which is clearly wrong in
this case.  That check was added in
<https://sourceware.org/ml/gdb-patches/2013-06/msg00028.html>, to
handle the case of sysroot being "remote:".  This patch fixes that
original issue in a different way.  Instead of checking whether the
path has a drive spec, check whether the sysroot is "remote:".  The
patch adds a table that helps visualize the cases that need a
separator.  I also confirmed the original issue is still handled as
expected.  That is, that "set sysroot remote:" still does the right
thing.

remote_filename_p returns true if the filename is prefixed with
"remote:".  In this case, we need to check whether the filename is
exactly "remote:".  I thought of different ways or either changing
remote_filename_p or adding another convenience function to remote.c
to avoid exposing the "remote:" prefix out of remote.c.  But all
attempts turned out adding lot of over needless complication.  So the
patch just exposes the prefix behind a new macro, which allows using a
straighforward strcmp.

gdb/
2013-09-27  Pedro Alves  <palves@redhat.com>

	* remote.h (REMOTE_SYSROOT_PREFIX): New define.
	(remote_filename_p): Add comment.
	* remote.c (remote_filename_p): Adjust to use
	REMOTE_SYSROOT_PREFIX.
	* solib.c (solib_find): When deciding whether we need to add a
	directory separator, check whether the sysroot is "remote:"
	instead of checking whether the patch has a drive spec.  Add
	comments.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 555dc3f55dfeeb64811cdb5cac661813791521cf..b777d5faa53f2df06554cd68ac9fd8e9e377d308 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2013-09-27  Pedro Alves  <palves@redhat.com>
+
+       * remote.h (REMOTE_SYSROOT_PREFIX): New define.
+       (remote_filename_p): Add comment.
+       * remote.c (remote_filename_p): Adjust to use
+       REMOTE_SYSROOT_PREFIX.
+       * solib.c (solib_find): When deciding whether we need to add a
+       directory separator, check whether the sysroot is "remote:"
+       instead of checking whether the patch has a drive spec.  Add
+       comments.
+
 2013-09-27  Pedro Alves  <palves@redhat.com>
 
        * remote.c (struct stop_reply) <solibs_changed, replay_event>:

diff --git a/gdb/remote.c b/gdb/remote.c
index 0fa1e2b029b79379ac0a9fd946c5370cd89f36be..a9ef2974ac34c854312b49339eeaee0c4ad2cede 100644 (file)
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10036,7 +10036,9 @@ remote_bfd_iovec_stat (struct bfd *abfd, void *stream, struct stat *sb)
 int
 remote_filename_p (const char *filename)
 {
-  return strncmp (filename, "remote:", 7) == 0;
+  return strncmp (filename,
+                 REMOTE_SYSROOT_PREFIX,
+                 sizeof (REMOTE_SYSROOT_PREFIX) - 1) == 0;
 }
 
 bfd *

diff --git a/gdb/remote.h b/gdb/remote.h
index c036db71bb82af329e6b47c80f37b979f274a2ea..b6ea5470d8ef21d5ec08eb9d54c0b6411bec9a17 100644 (file)
--- a/gdb/remote.h
+++ b/gdb/remote.h
@@ -57,6 +57,13 @@ void remote_file_delete (const char *remote_file, int from_tty);
 
 bfd *remote_bfd_open (const char *remote_file, const char *target);
 
+/* If a path starts with this sequence, GDB will retrieve the target
+   libraries from the remote system.  */
+
+#define REMOTE_SYSROOT_PREFIX "remote:"
+
+/* True if FILENAME starts with REMOTE_SYSROOT_PREFIX.  */
+
 int remote_filename_p (const char *filename);
 
 extern int remote_register_number_and_offset (struct gdbarch *gdbarch,

diff --git a/gdb/solib.c b/gdb/solib.c
index 2b3d45640010cbeaf8cdf58eec3182b065ecd45e..8ed492a4136f18e6de854e6d43ca6e7c0debf8a1 100644 (file)
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -230,8 +230,25 @@ solib_find (char *in_pathname, int *fd)
     {
       int need_dir_separator;
 
-      need_dir_separator = (!IS_DIR_SEPARATOR (in_pathname[0])
-                           && !HAS_TARGET_DRIVE_SPEC (fskind, in_pathname));
+      /* Concatenate the sysroot and the target reported filename.  We
+        may need to glue them with a directory separator.  Cases to
+        consider:
+
+        | sysroot         | separator | in_pathname    |
+        |-----------------+-----------+----------------|
+        | /some/dir       | /         | c:/foo/bar.dll |
+        | /some/dir       |           | /foo/bar.dll   |
+        | remote:         |           | c:/foo/bar.dll |
+        | remote:         |           | /foo/bar.dll   |
+        | remote:some/dir | /         | c:/foo/bar.dll |
+        | remote:some/dir |           | /foo/bar.dll   |
+
+       IOW, we don't need to add a separator if IN_PATHNAME already
+       has one, or when the the sysroot is exactly "remote:".
+       There's no need to check for drive spec explicitly, as we only
+       get here if IN_PATHNAME is considered an absolute path.  */
+      need_dir_separator = !(IS_DIR_SEPARATOR (in_pathname[0])
+                            || strcmp (REMOTE_SYSROOT_PREFIX, sysroot) == 0);
 
       /* Cat the prefixed pathname together.  */
       temp_pathname = concat (sysroot,