From 3736004f01ce839ed6f27fa97bdf8d5dea3e4653 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 27 Sep 2013 15:29:06 +0000 Subject: [PATCH] Fix regular /path/to/directory sysroots and target reported dll paths with drive specs. 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 , 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 * 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. --- gdb/ChangeLog | 11 +++++++++++ gdb/remote.c | 4 +++- gdb/remote.h | 7 +++++++ gdb/solib.c | 21 +++++++++++++++++++-- 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 555dc3f55d..b777d5faa5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2013-09-27 Pedro Alves + + * 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 * remote.c (struct stop_reply) : diff --git a/gdb/remote.c b/gdb/remote.c index 0fa1e2b029..a9ef2974ac 100644 --- 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 c036db71bb..b6ea5470d8 100644 --- 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 2b3d456400..8ed492a413 100644 --- 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, -- 2.34.1