From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1929 invoked by alias); 27 Sep 2013 12:58:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 1918 invoked by uid 89); 27 Sep 2013 12:58:06 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Sep 2013 12:58:06 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.5 required=5.0 tests=AWL,BAYES_40,RP_MATCHES_RCVD,SPAM_SUBJECT autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8RCw2Bw014509 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 27 Sep 2013 08:58:02 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r8RCw0GW011410; Fri, 27 Sep 2013 08:58:01 -0400 Message-ID: <524580D8.3090409@redhat.com> Date: Fri, 27 Sep 2013 12:58:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: [PATCH] Fix regular /path/to/directory sysroots and target reported dll paths with drive specs. (was: Re: [PATCH] Don't set dir separator if path has drive spec) References: <1370306007-877-1-git-send-email-yao@codesourcery.com> In-Reply-To: <1370306007-877-1-git-send-email-yao@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-09/txt/msg00953.txt.bz2 On 06/04/2013 01:33 AM, Yao Qi wrote: > Hi, > When I debug a remote Windows program on Linux host, set sysroot to > "remote:", find GDB can't load these dlls from the remote. If the dll > name is "C:/Windows/system32/ntdll.dll" and GDB add "remote:" prefix > to the path. The dll name becomes "remote:/C:/Windows/system32/ntdll.dll" > and GDBserver is confused by the name. The first slash shouldn't be > added. In solib.c:solib_find, > > need_dir_separator = !IS_DIR_SEPARATOR (in_pathname[0]); > > it controls whether to insert dir separator after sysroot. It works > for UNIX-like file path, but it doesn't work for the DOS-like absolute > path. This patch is to fix this problem. Is it OK? > > gdb: > > 2013-06-04 Yao Qi > > * solib.c (solib_find): Don't need dir separator if path has > drive spec. This actually broke sysroots that are _not_ "remote:". Comments on the patch below? ------------ 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/remote.c | 4 +++- gdb/remote.h | 7 +++++++ gdb/solib.c | 21 +++++++++++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 0fa1e2b..a9ef297 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 c036db7..b6ea547 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 5456d6f..5516b3e 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, -- 1.7.11.7