[PATCH 0/9] New default sysroot "target:"

[PATCH 0/9] New default sysroot "target:"

[Gary Benson]

Hi all,

This series introduces a new "target:" prefix to "set sysroot".  Files
specified with a "target:" prefix will be loaded via the target: from
the remote if the target is remote, and from the local file system
otherwise.  This new prefix replaces the "remote:" prefix, and the
final patch in the series makes it the default.

The way the "target:" prefix is implemented differs somewhat from the
way the "remote:" prefix was implemented:

 - It's hooked in at a lower level.  The remote stuff looked to have
   been added piecemeal: various BFD-opening functions did checks on
   their filenames and diverted to remote_bfd_open.  There was also
   gdb_bfd_open_maybe_remote, which handled both local and remote
   cases.  The "target:" prefix is baked into gdb_bfd_open, so all
   functions that open BFDs gain support.

 - Various functions locally strip the "target:" prefix from the
   filenames they're working on if the target filesystem is the
   same as the local filesystem.  This serves two purposes:

     1) It ensures files accessed locally are handled the same
        way regardless of how they are specified.  Things like
	the shared library search algorithm in solib_find, for
	example.

     2) It avoids cluttering GDB's output with "target:" prefixes.

Built and regtested on RHEL 6.6 x86_64.

Ok to commit?

Thanks,
Gary

--
http://gbenson.net/

[PATCH 1/9] Introduce target_fileio_fstat

[Gary Benson]

This commit introduces a new target method target_fileio_fstat
which can be used to retrieve information about files opened with
target_fileio_open.

gdb/ChangeLog:

	* target.h (struct target_ops) <to_fileio_fstat>: New field.
	(target_fileio_fstat): New declaration.
	* target.c (target_fileio_fstat): New function.
	* inf-child.c (inf_child_fileio_fstat): Likewise.
	(inf_child_target): Initialize to_fileio_fstat.
	* remote.c (init_remote_ops): Likewise.
---
 gdb/ChangeLog   |    9 +++++++++
 gdb/inf-child.c |   17 +++++++++++++++++
 gdb/remote.c    |    1 +
 gdb/target.c    |   21 +++++++++++++++++++++
 gdb/target.h    |   11 +++++++++++
 5 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/gdb/inf-child.c b/gdb/inf-child.c
index 494f4b8..a0b786c 100644
--- a/gdb/inf-child.c
+++ b/gdb/inf-child.c
@@ -374,6 +374,22 @@ inf_child_fileio_pread (struct target_ops *self,
   return ret;
 }
 
+/* Get information about the file opened as FD and put it in SB.
+   Return 0, or -1 if an error occurs (and set *TARGET_ERRNO).  */
+
+static int
+inf_child_fileio_fstat (struct target_ops *self, int fd,
+			struct stat *sb, int *target_errno)
+{
+  int ret;
+
+  ret = fstat (fd, sb);
+  if (ret == -1)
+    *target_errno = inf_child_errno_to_fileio_error (errno);
+
+  return ret;
+}
+
 /* Close FD on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
 static int
@@ -500,6 +516,7 @@ inf_child_target (void)
   t->to_fileio_open = inf_child_fileio_open;
   t->to_fileio_pwrite = inf_child_fileio_pwrite;
   t->to_fileio_pread = inf_child_fileio_pread;
+  t->to_fileio_fstat = inf_child_fileio_fstat;
   t->to_fileio_close = inf_child_fileio_close;
   t->to_fileio_unlink = inf_child_fileio_unlink;
   t->to_fileio_readlink = inf_child_fileio_readlink;
diff --git a/gdb/remote.c b/gdb/remote.c
index dfa68b3..9c6d2e5 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11844,6 +11844,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_fileio_open = remote_hostio_open;
   remote_ops.to_fileio_pwrite = remote_hostio_pwrite;
   remote_ops.to_fileio_pread = remote_hostio_pread;
+  remote_ops.to_fileio_fstat = remote_hostio_fstat;
   remote_ops.to_fileio_close = remote_hostio_close;
   remote_ops.to_fileio_unlink = remote_hostio_unlink;
   remote_ops.to_fileio_readlink = remote_hostio_readlink;
diff --git a/gdb/target.c b/gdb/target.c
index bb901b5..44ede10 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2848,6 +2848,27 @@ target_fileio_pread (int fd, gdb_byte *read_buf, int len,
   return ret;
 }
 
+/* Get information about the file opened as FD on the target
+   and put it in SB.  Return 0, or -1 if an error occurs (and
+   set *TARGET_ERRNO).  */
+int
+target_fileio_fstat (int fd, struct stat *sb, int *target_errno)
+{
+  fileio_fh_t *fh = fileio_fd_to_fh (fd);
+  int ret = -1;
+
+  if (is_closed_fileio_fh (fh->fd))
+    *target_errno = EBADF;
+  else
+    ret = fh->t->to_fileio_fstat (fh->t, fh->fd, sb, target_errno);
+
+  if (targetdebug)
+    fprintf_unfiltered (gdb_stdlog,
+			"target_fileio_fstat (%d) = %d (%d)\n",
+			fd, ret, ret != -1 ? 0 : *target_errno);
+  return ret;
+}
+
 /* Close FD on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
 int
diff --git a/gdb/target.h b/gdb/target.h
index c95e1a4..d2bd152 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -846,6 +846,11 @@ struct target_ops
 			    int fd, gdb_byte *read_buf, int len,
 			    ULONGEST offset, int *target_errno);
 
+    /* Get information about the file opened as FD and put it in SB.
+       Return 0, or -1 if an error occurs (and set *TARGET_ERRNO).  */
+    int (*to_fileio_fstat) (struct target_ops *,
+			    int fd, struct stat *sb, int *target_errno);
+
     /* Close FD on the target.  Return 0, or -1 if an error occurs
        (and set *TARGET_ERRNO).  */
     int (*to_fileio_close) (struct target_ops *, int fd, int *target_errno);
@@ -1933,6 +1938,12 @@ extern int target_fileio_pwrite (int fd, const gdb_byte *write_buf, int len,
 extern int target_fileio_pread (int fd, gdb_byte *read_buf, int len,
 				ULONGEST offset, int *target_errno);
 
+/* Get information about the file opened as FD on the target
+   and put it in SB.  Return 0, or -1 if an error occurs (and
+   set *TARGET_ERRNO).  */
+extern int target_fileio_fstat (int fd, struct stat *sb,
+				int *target_errno);
+
 /* Close FD on the target.  Return 0, or -1 if an error occurs
    (and set *TARGET_ERRNO).  */
 extern int target_fileio_close (int fd, int *target_errno);
-- 
1.7.1

Re: [PATCH 1/9] Introduce target_fileio_fstat

[Pedro Alves]

Looks good with the nits below addressed.

On 03/20/2015 04:47 PM, Gary Benson wrote:

> --- a/gdb/inf-child.c
> +++ b/gdb/inf-child.c
> @@ -374,6 +374,22 @@ inf_child_fileio_pread (struct target_ops *self,
>    return ret;
>  }
>  
> +/* Get information about the file opened as FD and put it in SB.
> +   Return 0, or -1 if an error occurs (and set *TARGET_ERRNO).  */

Write something like "Implementation of to_fileio_fstat."
instead of duplicating the comment.

> +
> +static int
> +inf_child_fileio_fstat (struct target_ops *self, int fd,
> +			struct stat *sb, int *target_errno)
> +{

> index bb901b5..44ede10 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -2848,6 +2848,27 @@ target_fileio_pread (int fd, gdb_byte *read_buf, int len,
>    return ret;
>  }
>  
> +/* Get information about the file opened as FD on the target
> +   and put it in SB.  Return 0, or -1 if an error occurs (and
> +   set *TARGET_ERRNO).  */

Write:

/* See target.h.  */

instead of duplicating the comment.

> +int
> +target_fileio_fstat (int fd, struct stat *sb, int *target_errno)
> +{
> +  fileio_fh_t *fh = fileio_fd_to_fh (fd);
> +  int ret = -1;
> +
> +  if (is_closed_fileio_fh (fh->fd))
> +    *target_errno = EBADF;
> +  else
> +    ret = fh->t->to_fileio_fstat (fh->t, fh->fd, sb, target_errno);
> +
> +  if (targetdebug)
> +    fprintf_unfiltered (gdb_stdlog,
> +			"target_fileio_fstat (%d) = %d (%d)\n",
> +			fd, ret, ret != -1 ? 0 : *target_errno);
> +  return ret;
> +}
> +
>  /* Close FD on the target.  Return 0, or -1 if an error occurs
>     (and set *TARGET_ERRNO).  */
>  int
> diff --git a/gdb/target.h b/gdb/target.h
> index c95e1a4..d2bd152 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -846,6 +846,11 @@ struct target_ops
>  			    int fd, gdb_byte *read_buf, int len,
>  			    ULONGEST offset, int *target_errno);
>  
> +    /* Get information about the file opened as FD and put it in SB.
> +       Return 0, or -1 if an error occurs (and set *TARGET_ERRNO).  */

ITYM, "Return 0 on success, ..."

> +    int (*to_fileio_fstat) (struct target_ops *,
> +			    int fd, struct stat *sb, int *target_errno);
> +

> +/* Get information about the file opened as FD on the target
> +   and put it in SB.  Return 0, or -1 if an error occurs (and

Likewise.

> +   set *TARGET_ERRNO).  */
> +extern int target_fileio_fstat (int fd, struct stat *sb,
> +				int *target_errno);

Thanks,
Pedro Alves

[PATCH 4/9] Convert "remote:" sysroots to "target:" and remove "remote:"

[Gary Benson]

The functionality of "target:" sysroots is a superset of the
functionality of "remote:" sysroots.  This commit causes the
"set sysroot" command to rewrite "remote:" sysroots as "target:"
sysroots and replaces "remote:" specific code with "target:"
specific code where still necessary.

gdb/ChangeLog:

	* remote.h (REMOTE_SYSROOT_PREFIX): Remove definition.
	(remote_filename_p): Remove declaration.
	(remote_bfd_open): Likewise.
	* remote.c (remote_bfd_iovec_open): Remove function.
	(remote_bfd_iovec_close): Likewise.
	(remote_bfd_iovec_pread): Likewise.
	(remote_bfd_iovec_stat): Likewise.
	(remote_filename_p): Likewise.
	(remote_bfd_open): Likewise.
	* symfile.h (gdb_bfd_open_maybe_remote): Remove declaration.
	* symfile.c (separate_debug_file_exists): Use gdb_bfd_open.
	(gdb_bfd_open_maybe_remote): Remove function.
	(symfile_bfd_open):  Replace remote filename check with
	target filename check.
	(reread_symbols): Use gdb_bfd_open.
	* build-id.c (gdbcore.h): New include.
	(build_id_to_debug_bfd): Use gdb_bfd_open.
	* infcmd.c (attach_command_post_wait): Remove remote filename
	check.
	* solib.c (solib_find): Replace remote-specific handling with
	target-specific handling.  Update comments where necessary.
	(solib_bfd_open): Replace remote-specific handling with
	target-specific handling.
	(gdb_sysroot_changed): New function.
	(_initialize_solib): Call the above when gdb_sysroot changes.
	* windows-tdep.c (gdbcore.h): New include.
	(windows_xfer_shared_library): Use gdb_bfd_open.
---
 gdb/ChangeLog      |   30 ++++++++++++++
 gdb/build-id.c     |    3 +-
 gdb/remote.c       |  107 ----------------------------------------------------
 gdb/remote.h       |   11 -----
 gdb/solib.c        |   53 +++++++++++++++-----------
 gdb/symfile.c      |   25 ++----------
 gdb/symfile.h      |    2 -
 gdb/windows-tdep.c |    3 +-
 8 files changed, 69 insertions(+), 165 deletions(-)

diff --git a/gdb/build-id.c b/gdb/build-id.c
index 3a6ebb1..8f7bbb4 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -26,6 +26,7 @@
 #include "symfile.h"
 #include "objfiles.h"
 #include "filenames.h"
+#include "gdbcore.h"
 
 /* See build-id.h.  */
 
@@ -118,7 +119,7 @@ build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id)
 	continue;
 
       /* We expect to be silent on the non-existing files.  */
-      abfd = gdb_bfd_open_maybe_remote (filename);
+      abfd = gdb_bfd_open (filename, gnutarget, -1);
       if (abfd == NULL)
 	continue;
 
diff --git a/gdb/remote.c b/gdb/remote.c
index c0fab92..2a4e019 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -10207,113 +10207,6 @@ remote_hostio_close_cleanup (void *opaque)
   remote_hostio_close (find_target_at (process_stratum), fd, &remote_errno);
 }
 
-
-static void *
-remote_bfd_iovec_open (struct bfd *abfd, void *open_closure)
-{
-  const char *filename = bfd_get_filename (abfd);
-  int fd, remote_errno;
-  int *stream;
-
-  gdb_assert (remote_filename_p (filename));
-
-  fd = remote_hostio_open (find_target_at (process_stratum),
-			   filename + 7, FILEIO_O_RDONLY, 0, &remote_errno);
-  if (fd == -1)
-    {
-      errno = remote_fileio_errno_to_host (remote_errno);
-      bfd_set_error (bfd_error_system_call);
-      return NULL;
-    }
-
-  stream = xmalloc (sizeof (int));
-  *stream = fd;
-  return stream;
-}
-
-static int
-remote_bfd_iovec_close (struct bfd *abfd, void *stream)
-{
-  int fd = *(int *)stream;
-  int remote_errno;
-
-  xfree (stream);
-
-  /* Ignore errors on close; these may happen if the remote
-     connection was already torn down.  */
-  remote_hostio_close (find_target_at (process_stratum), fd, &remote_errno);
-
-  /* Zero means success.  */
-  return 0;
-}
-
-static file_ptr
-remote_bfd_iovec_pread (struct bfd *abfd, void *stream, void *buf,
-			file_ptr nbytes, file_ptr offset)
-{
-  int fd = *(int *)stream;
-  int remote_errno;
-  file_ptr pos, bytes;
-
-  pos = 0;
-  while (nbytes > pos)
-    {
-      bytes = remote_hostio_pread (find_target_at (process_stratum),
-				   fd, (gdb_byte *) buf + pos, nbytes - pos,
-				   offset + pos, &remote_errno);
-      if (bytes == 0)
-        /* Success, but no bytes, means end-of-file.  */
-        break;
-      if (bytes == -1)
-	{
-	  errno = remote_fileio_errno_to_host (remote_errno);
-	  bfd_set_error (bfd_error_system_call);
-	  return -1;
-	}
-
-      pos += bytes;
-    }
-
-  return pos;
-}
-
-static int
-remote_bfd_iovec_stat (struct bfd *abfd, void *stream, struct stat *sb)
-{
-  int fd = *(int *) stream;
-  int remote_errno;
-  int result;
-
-  result = remote_hostio_fstat (find_target_at (process_stratum),
-				fd, sb, &remote_errno);
-
-  if (result == -1)
-    {
-      errno = remote_fileio_errno_to_host (remote_errno);
-      bfd_set_error (bfd_error_system_call);
-    }
-
-  return result;
-}
-
-int
-remote_filename_p (const char *filename)
-{
-  return startswith (filename, REMOTE_SYSROOT_PREFIX);
-}
-
-bfd *
-remote_bfd_open (const char *remote_file, const char *target)
-{
-  bfd *abfd = gdb_bfd_openr_iovec (remote_file, target,
-				   remote_bfd_iovec_open, NULL,
-				   remote_bfd_iovec_pread,
-				   remote_bfd_iovec_close,
-				   remote_bfd_iovec_stat);
-
-  return abfd;
-}
-
 void
 remote_file_put (const char *local_file, const char *remote_file, int from_tty)
 {
diff --git a/gdb/remote.h b/gdb/remote.h
index e647537..fb2a72b 100644
--- a/gdb/remote.h
+++ b/gdb/remote.h
@@ -49,17 +49,6 @@ void remote_file_get (const char *remote_file, const char *local_file,
 		      int from_tty);
 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,
 					      int regnum, int *pnum,
 					      int *poffset);
diff --git a/gdb/solib.c b/gdb/solib.c
index c8138ef..039f529 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -236,17 +236,17 @@ solib_find (char *in_pathname, int *fd)
         |-----------------+-----------+----------------|
         | /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   |
+        | target:         |           | c:/foo/bar.dll |
+        | target:         |           | /foo/bar.dll   |
+        | target:some/dir | /         | c:/foo/bar.dll |
+        | target: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:".
+	has one, or when the the sysroot is exactly "target:".
 	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);
+			     || strcmp (TARGET_SYSROOT_PREFIX, sysroot) == 0);
 
       /* Cat the prefixed pathname together.  */
       temp_pathname = concat (sysroot,
@@ -254,8 +254,8 @@ solib_find (char *in_pathname, int *fd)
 			      in_pathname, (char *) NULL);
     }
 
-  /* Handle remote files.  */
-  if (remote_filename_p (temp_pathname))
+  /* Handle files to be accessed via the target.  */
+  if (is_target_filename (temp_pathname))
     {
       *fd = -1;
       do_cleanups (old_chain);
@@ -382,20 +382,10 @@ solib_find (char *in_pathname, int *fd)
 bfd *
 solib_bfd_fopen (char *pathname, int fd)
 {
-  bfd *abfd;
+  bfd *abfd = gdb_bfd_open (pathname, gnutarget, fd);
 
-  if (remote_filename_p (pathname))
-    {
-      gdb_assert (fd == -1);
-      abfd = remote_bfd_open (pathname, gnutarget);
-    }
-  else
-    {
-      abfd = gdb_bfd_open (pathname, gnutarget, fd);
-
-      if (abfd)
-	bfd_set_cacheable (abfd, 1);
-    }
+  if (abfd != NULL && !gdb_bfd_has_target_filename (abfd))
+    bfd_set_cacheable (abfd, 1);
 
   if (!abfd)
     {
@@ -1403,6 +1393,25 @@ reload_shared_libraries (char *ignored, int from_tty,
   ops->special_symbol_handling ();
 }
 
+/* Wrapper for reload_shared_libraries that replaces "remote:"
+   at the start of gdb_sysroot with "target:".  */
+
+static void
+gdb_sysroot_changed (char *ignored, int from_tty,
+		     struct cmd_list_element *e)
+{
+  const char *old_prefix = "remote:";
+  const char *new_prefix = TARGET_SYSROOT_PREFIX;
+
+  if (startswith (gdb_sysroot, old_prefix))
+    {
+      gdb_assert (strlen (old_prefix) == strlen (new_prefix));
+      memcpy (gdb_sysroot, new_prefix, strlen (new_prefix));
+    }
+
+  reload_shared_libraries (ignored, from_tty, e);
+}
+
 static void
 show_auto_solib_add (struct ui_file *file, int from_tty,
 		     struct cmd_list_element *c, const char *value)
@@ -1597,7 +1606,7 @@ Show the current system root."), _("\
 The system root is used to load absolute shared library symbol files.\n\
 For other (relative) files, you can add directories using\n\
 `set solib-search-path'."),
-				     reload_shared_libraries,
+				     gdb_sysroot_changed,
 				     NULL,
 				     &setlist, &showlist);
 
diff --git a/gdb/symfile.c b/gdb/symfile.c
index ba099d3..0d8dae7 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1368,7 +1368,7 @@ separate_debug_file_exists (const char *name, unsigned long crc,
   if (filename_cmp (name, objfile_name (parent_objfile)) == 0)
     return 0;
 
-  abfd = gdb_bfd_open_maybe_remote (name);
+  abfd = gdb_bfd_open (name, gnutarget, -1);
 
   if (!abfd)
     return 0;
@@ -1712,23 +1712,6 @@ set_initial_language (void)
   expected_language = current_language; /* Don't warn the user.  */
 }
 
-/* If NAME is a remote name open the file using remote protocol, otherwise
-   open it normally.  Returns a new reference to the BFD.  On error,
-   returns NULL with the BFD error set.  */
-
-bfd *
-gdb_bfd_open_maybe_remote (const char *name)
-{
-  bfd *result;
-
-  if (remote_filename_p (name))
-    result = remote_bfd_open (name, gnutarget);
-  else
-    result = gdb_bfd_open (name, gnutarget, -1);
-
-  return result;
-}
-
 /* Open the file specified by NAME and hand it off to BFD for
    preliminary analysis.  Return a newly initialized bfd *, which
    includes a newly malloc'd` copy of NAME (tilde-expanded and made
@@ -1742,9 +1725,9 @@ symfile_bfd_open (const char *cname)
   char *name, *absolute_name;
   struct cleanup *back_to;
 
-  if (remote_filename_p (cname))
+  if (is_target_filename (cname))
     {
-      sym_bfd = remote_bfd_open (cname, gnutarget);
+      sym_bfd = gdb_bfd_open (cname, gnutarget, -1);
       if (!sym_bfd)
 	error (_("`%s': can't open to read symbols: %s."), cname,
 	       bfd_errmsg (bfd_get_error ()));
@@ -2596,7 +2579,7 @@ reread_symbols (void)
 	    obfd_filename = bfd_get_filename (objfile->obfd);
 	    /* Open the new BFD before freeing the old one, so that
 	       the filename remains live.  */
-	    objfile->obfd = gdb_bfd_open_maybe_remote (obfd_filename);
+	    objfile->obfd = gdb_bfd_open (obfd_filename, gnutarget, -1);
 	    if (objfile->obfd == NULL)
 	      {
 		/* We have to make a cleanup and error here, rather
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 7b66c62..9ef3f0b 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -514,8 +514,6 @@ extern void find_lowest_section (bfd *, asection *, void *);
 
 extern bfd *symfile_bfd_open (const char *);
 
-extern bfd *gdb_bfd_open_maybe_remote (const char *);
-
 extern int get_section_index (struct objfile *, char *);
 
 extern int print_symbol_loading_p (int from_tty, int mainline, int full);
diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index fbdddc9..dc4e2e4 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -33,6 +33,7 @@
 #include "complaints.h"
 #include "solib.h"
 #include "solib-target.h"
+#include "gdbcore.h"
 
 struct cmd_list_element *info_w32_cmdlist;
 
@@ -401,7 +402,7 @@ windows_xfer_shared_library (const char* so_name, CORE_ADDR load_addr,
   obstack_grow_str (obstack, p);
   xfree (p);
   obstack_grow_str (obstack, "\"><segment address=\"");
-  dll = gdb_bfd_open_maybe_remote (so_name);
+  dll = gdb_bfd_open (so_name, gnutarget, -1);
   /* The following calls are OK even if dll is NULL.
      The default value 0x1000 is returned by pe_text_section_offset
      in that case.  */
-- 
1.7.1

Re: [PATCH 4/9] Convert "remote:" sysroots to "target:" and remove "remote:"

[Pedro Alves]

Looks good.

On 03/20/2015 04:48 PM, Gary Benson wrote:
> +/* Wrapper for reload_shared_libraries that replaces "remote:"
> +   at the start of gdb_sysroot with "target:".  */
> +
> +static void
> +gdb_sysroot_changed (char *ignored, int from_tty,
> +		     struct cmd_list_element *e)
> +{
> +  const char *old_prefix = "remote:";
> +  const char *new_prefix = TARGET_SYSROOT_PREFIX;
> +
> +  if (startswith (gdb_sysroot, old_prefix))
> +    {
> +      gdb_assert (strlen (old_prefix) == strlen (new_prefix));
> +      memcpy (gdb_sysroot, new_prefix, strlen (new_prefix));

Should this output a warning (once per gdb invocation)?

> +    }
> +
> +  reload_shared_libraries (ignored, from_tty, e);
> +}
> +


Thanks,
Pedro Alves

Re: [PATCH 4/9] Convert "remote:" sysroots to "target:" and remove "remote:"

[Gary Benson]

Pedro Alves wrote:
> Looks good.
> 
> On 03/20/2015 04:48 PM, Gary Benson wrote:
> > +/* Wrapper for reload_shared_libraries that replaces "remote:"
> > +   at the start of gdb_sysroot with "target:".  */
> > +
> > +static void
> > +gdb_sysroot_changed (char *ignored, int from_tty,
> > +		     struct cmd_list_element *e)
> > +{
> > +  const char *old_prefix = "remote:";
> > +  const char *new_prefix = TARGET_SYSROOT_PREFIX;
> > +
> > +  if (startswith (gdb_sysroot, old_prefix))
> > +    {
> > +      gdb_assert (strlen (old_prefix) == strlen (new_prefix));
> > +      memcpy (gdb_sysroot, new_prefix, strlen (new_prefix));
> 
> Should this output a warning (once per gdb invocation)?

Sure, I'll add one.

Cheers,
Gary

-- 
http://gbenson.net/

[PATCH 6/9] Strip "target:" prefix in solib_find if accessing local files

[Gary Benson]

This commit updates solib_find to strip the "target:" prefix from
gdb_sysroot when accessing local files.  This ensures that the same
search algorithm is used for local files regardless of whether a
"target:" prefix was used or not.  It also avoids cluttering GDB's
output with unnecessary "target:" prefixes on paths.

gdb/ChangeLog:

	* solib.c (solib_find): Strip "target:" prefix from sysroot
	if accessing local files.
---
 gdb/ChangeLog |    5 +++++
 gdb/solib.c   |   51 +++++++++++++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/gdb/solib.c b/gdb/solib.c
index 039f529..359a208 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -119,24 +119,30 @@ show_solib_search_path (struct ui_file *file, int from_tty,
 
    Global variable GDB_SYSROOT is used as a prefix directory
    to search for shared libraries if they have an absolute path.
+   If GDB_SYSROOT starts with "target:" and target filesystem
+   is the local filesystem then the "target:" prefix will be
+   stripped before the search starts.  This ensures that the
+   same search algorithm is used for local files regardless of
+   whether a "target:" prefix was used.
 
    Global variable SOLIB_SEARCH_PATH is used as a prefix directory
    (or set of directories, as in LD_LIBRARY_PATH) to search for all
-   shared libraries if not found in GDB_SYSROOT.
+   shared libraries if not found in either the sysroot (if set) or
+   the local filesystem.
 
    Search algorithm:
-   * If there is a gdb_sysroot and path is absolute:
-   *   Search for gdb_sysroot/path.
+   * If a sysroot is set and path is absolute:
+   *   Search for sysroot/path.
    * else
    *   Look for it literally (unmodified).
    * Look in SOLIB_SEARCH_PATH.
    * If available, use target defined search function.
-   * If gdb_sysroot is NOT set, perform the following two searches:
+   * If NO sysroot is set, perform the following two searches:
    *   Look in inferior's $PATH.
    *   Look in inferior's $LD_LIBRARY_PATH.
    *
    * The last check avoids doing this search when targetting remote
-   * machines since gdb_sysroot will almost always be set.
+   * machines since a sysroot will almost always be set.
 */
 
 char *
@@ -145,12 +151,11 @@ solib_find (char *in_pathname, int *fd)
   const struct target_so_ops *ops = solib_ops (target_gdbarch ());
   int found_file = -1;
   char *temp_pathname = NULL;
-  int gdb_sysroot_is_empty;
   const char *solib_symbols_extension
     = gdbarch_solib_symbols_extension (target_gdbarch ());
   const char *fskind = effective_target_file_system_kind ();
   struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-  char *sysroot = NULL;
+  char *sysroot = gdb_sysroot;
 
   /* If solib_symbols_extension is set, replace the file's
      extension.  */
@@ -175,18 +180,32 @@ solib_find (char *in_pathname, int *fd)
 	}
     }
 
-  gdb_sysroot_is_empty = (gdb_sysroot == NULL || *gdb_sysroot == 0);
+  if (sysroot != NULL)
+    {
+      /* If the absolute prefix starts with "target:" but the
+	 filesystem accessed by the target_fileio_* methods
+	 is the local filesystem then we strip the "target:"
+	 prefix now and work with the local filesystem.  This
+	 ensures that the same search algorithm is used for
+	 all local files regardless of whether a "target:"
+	 prefix was used.  */
+      if (is_target_filename (sysroot) && target_filesystem_is_local)
+	sysroot += strlen (TARGET_SYSROOT_PREFIX);
+
+      if (*sysroot == '\0')
+	sysroot = NULL;
+    }
 
-  if (!gdb_sysroot_is_empty)
+  if (sysroot != NULL)
     {
-      int prefix_len = strlen (gdb_sysroot);
+      int prefix_len = strlen (sysroot);
 
       /* Remove trailing slashes from absolute prefix.  */
       while (prefix_len > 0
-	     && IS_DIR_SEPARATOR (gdb_sysroot[prefix_len - 1]))
+	     && IS_DIR_SEPARATOR (sysroot[prefix_len - 1]))
 	prefix_len--;
 
-      sysroot = savestring (gdb_sysroot, prefix_len);
+      sysroot = savestring (sysroot, prefix_len);
       make_cleanup (xfree, sysroot);
     }
 
@@ -222,7 +241,7 @@ solib_find (char *in_pathname, int *fd)
        3rd attempt, c:/foo/bar.dll ==> /sysroot/foo/bar.dll
   */
 
-  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || gdb_sysroot_is_empty)
+  if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
     temp_pathname = xstrdup (in_pathname);
   else
     {
@@ -273,7 +292,7 @@ solib_find (char *in_pathname, int *fd)
        c:/foo/bar.dll ==> /sysroot/c/foo/bar.dll.  */
 
   if (found_file < 0
-      && !gdb_sysroot_is_empty
+      && sysroot != NULL
       && HAS_TARGET_DRIVE_SPEC (fskind, in_pathname))
     {
       int need_dir_separator = !IS_DIR_SEPARATOR (in_pathname[2]);
@@ -353,7 +372,7 @@ solib_find (char *in_pathname, int *fd)
 					   &temp_pathname);
 
   /* If not found, next search the inferior's $PATH environment variable.  */
-  if (found_file < 0 && gdb_sysroot_is_empty)
+  if (found_file < 0 && sysroot == NULL)
     found_file = openp (get_in_environ (current_inferior ()->environment,
 					"PATH"),
 			OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, in_pathname,
@@ -361,7 +380,7 @@ solib_find (char *in_pathname, int *fd)
 
   /* If not found, next search the inferior's $LD_LIBRARY_PATH
      environment variable.  */
-  if (found_file < 0 && gdb_sysroot_is_empty)
+  if (found_file < 0 && sysroot == NULL)
     found_file = openp (get_in_environ (current_inferior ()->environment,
 					"LD_LIBRARY_PATH"),
 			OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, in_pathname,
-- 
1.7.1

Re: [PATCH 6/9] Strip "target:" prefix in solib_find if accessing local files

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:
> This commit updates solib_find to strip the "target:" prefix from
> gdb_sysroot when accessing local files.  This ensures that the same
> search algorithm is used for local files regardless of whether a
> "target:" prefix was used or not.  It also avoids cluttering GDB's
> output with unnecessary "target:" prefixes on paths.

OK.

Thanks,
Pedro Alves

[PATCH 9/9] Document "target:" sysroot changes

[Gary Benson]

This commit documents the newly added "target:" sysroot feature.

gdb/ChangeLog:

	* NEWS: Announce the new default sysroot of "target:".

gdb/doc/ChangeLog:

	* gdb.texinfo (set sysroot): Document "target:".
---
 gdb/ChangeLog       |    4 ++++
 gdb/NEWS            |    5 +++++
 gdb/doc/ChangeLog   |    4 ++++
 gdb/doc/gdb.texinfo |   23 ++++++++++++++---------
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index bda4a35..3d4f98d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -7,6 +7,11 @@
   present in the debug info.  This typically includes the compiler version
   and may include things like its command line arguments.
 
+* Paths supplied to the "set sysroot" commands may be prefixed with
+  "target:" to tell GDB to access shared libraries from the target
+  system, be it local or remote.  This replaces the prefix "remote:".
+  The default sysroot has been changed from "" to "target:".
+
 * Python Scripting
 
   ** gdb.Objfile objects have a new attribute "username",
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 552da31..c5c3724 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -17814,15 +17814,20 @@ libraries, they need to be laid out in the same way that they are on
 the target, with e.g.@: a @file{/lib} and @file{/usr/lib} hierarchy
 under @var{path}.
 
-If @var{path} starts with the sequence @file{remote:}, @value{GDBN} will 
-retrieve the target libraries from the remote system.  This is only
-supported when using a remote target that supports the @code{remote get}
-command (@pxref{File Transfer,,Sending files to a remote system}).
-The part of @var{path} following the initial @file{remote:}
-(if present) is used as system root prefix on the remote file system.
-@footnote{If you want to specify a local system root using a directory
-that happens to be named @file{remote:}, you need to use some equivalent
-variant of the name like @file{./remote:}.}
+If @var{path} starts with the sequence @file{target:} and the target
+system is remote then @value{GDBN} will retrieve the target binaries
+from the remote system.  This is only supported when using a remote
+target that supports the @code{remote get} command (@pxref{File
+Transfer,,Sending files to a remote system}).  The part of @var{path}
+following the initial @file{target:} (if present) is used as system
+root prefix on the remote file system.  If @var{path} starts with the
+sequence @file{remote:} this will be converted to the sequence
+@file{target:} by @code{set sysroot}. @footnote{Historically the
+functionality to retrieve binaries from the remote system was
+provided by prefixing @var{path} with @file{remote:}} @footnote{If you
+want to specify a local system root using a directory that happens to
+be named @file{target:} or @file{remote:}, you need to use some
+equivalent variant of the name like @file{./target:}.}
 
 For targets with an MS-DOS based filesystem, such as MS-Windows and
 SymbianOS, @value{GDBN} tries prefixing a few variants of the target
-- 
1.7.1

Re: [PATCH 9/9] Document "target:" sysroot changes

[Eli Zaretskii]

> From: Gary Benson <gbenson@redhat.com>
> Date: Fri, 20 Mar 2015 16:48:07 +0000
> 
> This commit documents the newly added "target:" sysroot feature.

Thanks.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -7,6 +7,11 @@
>    present in the debug info.  This typically includes the compiler version
>    and may include things like its command line arguments.
>  
> +* Paths supplied to the "set sysroot" commands may be prefixed with
> +  "target:" to tell GDB to access shared libraries from the target
> +  system, be it local or remote.  This replaces the prefix "remote:".
> +  The default sysroot has been changed from "" to "target:".

The Gnu Coding Standards frown on using "path" for anything but
PATH-style directory lists.  Please use "directory name" instead.

> +root prefix on the remote file system.  If @var{path} starts with the
> +sequence @file{remote:} this will be converted to the sequence
> +@file{target:} by @code{set sysroot}. @footnote{Historically the

The @footnote should be before the period, and without any whitespace
before it.

> +functionality to retrieve binaries from the remote system was
> +provided by prefixing @var{path} with @file{remote:}} @footnote{If you
> +want to specify a local system root using a directory that happens to
> +be named @file{target:} or @file{remote:}, you need to use some
> +equivalent variant of the name like @file{./target:}.}

The second footnote should not be a footnote, as it is an important
part of the description.

Thanks.

Re: [PATCH 9/9] Document "target:" sysroot changes

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:
> This commit documents the newly added "target:" sysroot feature.
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Announce the new default sysroot of "target:".
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (set sysroot): Document "target:".
> ---
>  gdb/ChangeLog       |    4 ++++
>  gdb/NEWS            |    5 +++++
>  gdb/doc/ChangeLog   |    4 ++++
>  gdb/doc/gdb.texinfo |   23 ++++++++++++++---------
>  4 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index bda4a35..3d4f98d 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -7,6 +7,11 @@
>    present in the debug info.  This typically includes the compiler version
>    and may include things like its command line arguments.
>  
> +* Paths supplied to the "set sysroot" commands may be prefixed with
> +  "target:" to tell GDB to access shared libraries from the target
> +  system, be it local or remote.  This replaces the prefix "remote:".
> +  The default sysroot has been changed from "" to "target:".

I think you should mention also what happens if you still use "remote:".
Something around '"remote:" is automatically converted to "target:" for
backward compatibility'.

> +If @var{path} starts with the sequence @file{target:} and the target
> +system is remote then @value{GDBN} will retrieve the target binaries
> +from the remote system.  This is only supported when using a remote
> +target that supports the @code{remote get} command (@pxref{File
> +Transfer,,Sending files to a remote system}).  The part of @var{path}
> +following the initial @file{target:} (if present) is used as system
> +root prefix on the remote file system.  If @var{path} starts with the
> +sequence @file{remote:} this will be converted to the sequence

s/this will be converted/this is converted/

> +@file{target:} by @code{set sysroot}. @footnote{Historically the
> +functionality to retrieve binaries from the remote system was
> +provided by prefixing @var{path} with @file{remote:}} @footnote{If you
> +want to specify a local system root using a directory that happens to
> +be named @file{target:} or @file{remote:}, you need to use some
> +equivalent variant of the name like @file{./target:}.}
>  
>  For targets with an MS-DOS based filesystem, such as MS-Windows and
>  SymbianOS, @value{GDBN} tries prefixing a few variants of the target
> 


Thanks,
Pedro Alves

[PATCH 8/9] Make the default sysroot be "target:"

[Gary Benson]

This commit makes GDB default to a sysroot of "target:".
One testcase needed updating as a result of this change.

gdb/ChangeLog:

	* main.c (captured_main): Set gdb_sysroot to "target:"
	if not otherwise set.

gdb/testsuite/ChangeLog:

	* gdb.base/break-probes.exp: Cope with "target:" sysroot.
---
 gdb/ChangeLog                           |    5 +++++
 gdb/main.c                              |    6 ++++++
 gdb/testsuite/ChangeLog                 |    4 ++++
 gdb/testsuite/gdb.base/break-probes.exp |    2 +-
 4 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 72b8714..8f276b4 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -548,6 +548,12 @@ captured_main (void *data)
   gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
 					TARGET_SYSTEM_ROOT_RELOCATABLE);
 
+  if (gdb_sysroot == NULL || *gdb_sysroot == '\0')
+    {
+      xfree (gdb_sysroot);
+      gdb_sysroot = xstrdup (TARGET_SYSROOT_PREFIX);
+    }
+
   debug_file_directory = relocate_gdb_directory (DEBUGDIR,
 						 DEBUGDIR_RELOCATABLE);
 
diff --git a/gdb/testsuite/gdb.base/break-probes.exp b/gdb/testsuite/gdb.base/break-probes.exp
index 1591c33..d4d756f 100644
--- a/gdb/testsuite/gdb.base/break-probes.exp
+++ b/gdb/testsuite/gdb.base/break-probes.exp
@@ -72,7 +72,7 @@ if { $using_probes } {
     while { $not_loaded_library } {
 	set not_loaded_library 0
 	gdb_test_multiple "c" $test {
-	    -re "Inferior loaded $sysroot$binfile_lib\\M.*$gdb_prompt $" {
+	    -re "Inferior loaded ($sysroot)?$binfile_lib\\M.*$gdb_prompt $" {
 		pass $test
 	    }
 	    -re "Stopped due to shared library event\\M.*$gdb_prompt $" {
-- 
1.7.1

Re: [PATCH 8/9] Make the default sysroot be "target:"

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:
> This commit makes GDB default to a sysroot of "target:".
> One testcase needed updating as a result of this change.

That's because when the sysroot is "target:" and the filesystem
is remote, the sysroot is stripped from the "Inferior loaded"
message, right?  A short comment in the test would help.

OK with that change.

Thanks,
Pedro Alves

> 
> gdb/ChangeLog:
> 
> 	* main.c (captured_main): Set gdb_sysroot to "target:"
> 	if not otherwise set.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/break-probes.exp: Cope with "target:" sysroot.
> ---
>  gdb/ChangeLog                           |    5 +++++
>  gdb/main.c                              |    6 ++++++
>  gdb/testsuite/ChangeLog                 |    4 ++++
>  gdb/testsuite/gdb.base/break-probes.exp |    2 +-
>  4 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/gdb/main.c b/gdb/main.c
> index 72b8714..8f276b4 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -548,6 +548,12 @@ captured_main (void *data)
>    gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
>  					TARGET_SYSTEM_ROOT_RELOCATABLE);
>  
> +  if (gdb_sysroot == NULL || *gdb_sysroot == '\0')
> +    {
> +      xfree (gdb_sysroot);
> +      gdb_sysroot = xstrdup (TARGET_SYSROOT_PREFIX);
> +    }
> +
>    debug_file_directory = relocate_gdb_directory (DEBUGDIR,
>  						 DEBUGDIR_RELOCATABLE);
>  
> diff --git a/gdb/testsuite/gdb.base/break-probes.exp b/gdb/testsuite/gdb.base/break-probes.exp
> index 1591c33..d4d756f 100644
> --- a/gdb/testsuite/gdb.base/break-probes.exp
> +++ b/gdb/testsuite/gdb.base/break-probes.exp
> @@ -72,7 +72,7 @@ if { $using_probes } {
>      while { $not_loaded_library } {
>  	set not_loaded_library 0
>  	gdb_test_multiple "c" $test {
> -	    -re "Inferior loaded $sysroot$binfile_lib\\M.*$gdb_prompt $" {
> +	    -re "Inferior loaded ($sysroot)?$binfile_lib\\M.*$gdb_prompt $" {
>  		pass $test
>  	    }
>  	    -re "Stopped due to shared library event\\M.*$gdb_prompt $" {

Re: [PATCH 8/9] Make the default sysroot be "target:"

[Gary Benson]

Pedro Alves wrote:
> On 03/20/2015 04:48 PM, Gary Benson wrote:
> > This commit makes GDB default to a sysroot of "target:".
> > One testcase needed updating as a result of this change.
> 
> That's because when the sysroot is "target:" and the filesystem is
> remote, the sysroot is stripped from the "Inferior loaded" message,
> right?  A short comment in the test would help.
> 
> OK with that change.

I reorganized this a little while adding the comment.
I'll commit this:

diff --git a/gdb/testsuite/gdb.base/break-probes.exp...
index 1591c33..23aca8f 100644
--- a/gdb/testsuite/gdb.base/break-probes.exp
+++ b/gdb/testsuite/gdb.base/break-probes.exp
@@ -66,6 +66,10 @@ if { $using_probes } {
 	}
     }
 
+    # GDB strips "target:" from the start of filenames when
+    # operating on the local filesystem
+    regsub "^target:" "$sysroot" "(target:)?" sysroot
+
     # Run til it loads our library
     set test "run til our library loads"
     set not_loaded_library 1

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 8/9] Make the default sysroot be "target:"

[Pedro Alves]

On 04/01/2015 03:12 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 03/20/2015 04:48 PM, Gary Benson wrote:
>>> This commit makes GDB default to a sysroot of "target:".
>>> One testcase needed updating as a result of this change.
>>
>> That's because when the sysroot is "target:" and the filesystem is
>> remote, the sysroot is stripped from the "Inferior loaded" message,
>> right?  A short comment in the test would help.
>>
>> OK with that change.
> 
> I reorganized this a little while adding the comment.
> I'll commit this:

Thanks, even better.

Thanks,
Pedro Alves

[PATCH 7/9] Update exec_file_attach to cope with "target:" filenames

[Gary Benson]

This commit adds support for filenames prefixed	with "target:" to
exec_file_attach.  This is required to correctly follow inferior
exec* calls when a gdb_sysroot prefixed with "target:" is set.

gdb/ChangeLog:

	* exec.c (exec_file_attach): Support "target:" filenames.
---
 gdb/ChangeLog |    4 +++
 gdb/exec.c    |   71 +++++++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index 124074f..113e869 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -176,36 +176,66 @@ exec_file_attach (const char *filename, int from_tty)
     }
   else
     {
+      int load_via_target = 0;
       char *scratch_pathname, *canonical_pathname;
       int scratch_chan;
       struct target_section *sections = NULL, *sections_end = NULL;
       char **matching;
 
-      scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
-		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
-			    &scratch_pathname);
-#if defined(__GO32__) || defined(_WIN32) || defined(__CYGWIN__)
-      if (scratch_chan < 0)
+      if (is_target_filename (filename))
+	{
+	  if (target_filesystem_is_local)
+	    filename += strlen (TARGET_SYSROOT_PREFIX);
+	  else
+	    load_via_target = 1;
+	}
+
+      if (load_via_target)
 	{
-	  char *exename = alloca (strlen (filename) + 5);
+	  if (write_files)
+	    warning (_("writing into executable files is "
+		       "not supported for %s sysroots"),
+		     TARGET_SYSROOT_PREFIX);
+
+	  scratch_pathname = xstrdup (filename);
+	  make_cleanup (xfree, scratch_pathname);
 
-	  strcat (strcpy (exename, filename), ".exe");
-	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, exename,
-	     write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
-	     &scratch_pathname);
+	  scratch_chan = -1;
+
+	  canonical_pathname = scratch_pathname;
 	}
+      else
+	{
+
+	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST,
+				filename, write_files ?
+				O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
+				&scratch_pathname);
+#if defined(__GO32__) || defined(_WIN32) || defined(__CYGWIN__)
+	  if (scratch_chan < 0)
+	    {
+	      char *exename = alloca (strlen (filename) + 5);
+
+	      strcat (strcpy (exename, filename), ".exe");
+	      scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST,
+				    exename, write_files ?
+				    O_RDWR | O_BINARY
+				    : O_RDONLY | O_BINARY,
+				    &scratch_pathname);
+	    }
 #endif
-      if (scratch_chan < 0)
-	perror_with_name (filename);
+	  if (scratch_chan < 0)
+	    perror_with_name (filename);
 
-      make_cleanup (xfree, scratch_pathname);
+	  make_cleanup (xfree, scratch_pathname);
 
-      /* gdb_bfd_open (and its variants) prefers canonicalized pathname for
-	 better BFD caching.  */
-      canonical_pathname = gdb_realpath (scratch_pathname);
-      make_cleanup (xfree, canonical_pathname);
+	  /* gdb_bfd_open (and its variants) prefers canonicalized
+	     pathname for better BFD caching.  */
+	  canonical_pathname = gdb_realpath (scratch_pathname);
+	  make_cleanup (xfree, canonical_pathname);
+	}
 
-      if (write_files)
+      if (write_files && !load_via_target)
 	exec_bfd = gdb_bfd_fopen (canonical_pathname, gnutarget,
 				  FOPEN_RUB, scratch_chan);
       else
@@ -218,7 +248,10 @@ exec_file_attach (const char *filename, int from_tty)
 	}
 
       gdb_assert (exec_filename == NULL);
-      exec_filename = gdb_realpath_keepfile (scratch_pathname);
+      if (load_via_target)
+	exec_filename = xstrdup (bfd_get_filename (exec_bfd));
+      else
+	exec_filename = gdb_realpath_keepfile (scratch_pathname);
 
       if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
 	{
-- 
1.7.1

Re: [PATCH 7/9] Update exec_file_attach to cope with "target:" filenames

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:
> This commit adds support for filenames prefixed	with "target:" to
> exec_file_attach.  This is required to correctly follow inferior
> exec* calls when a gdb_sysroot prefixed with "target:" is set.

Hmm, I don't see how.  Isn't this only true when target_pid_to_exec_file
prepends the sysroot, which it doesn't yet?  I think this should move
to that other series.

A couple bits could use more explanation (in commit log and/or
comments):

 - Why is writing into executable files not supported with "target:" ?
 - The skipping of gdb_realpath_keepfile.

Thanks,
Pedro Alves

> 
> gdb/ChangeLog:
> 
> 	* exec.c (exec_file_attach): Support "target:" filenames.
> ---
>  gdb/ChangeLog |    4 +++
>  gdb/exec.c    |   71 +++++++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 56 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 124074f..113e869 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -176,36 +176,66 @@ exec_file_attach (const char *filename, int from_tty)
>      }
>    else
>      {
> +      int load_via_target = 0;
>        char *scratch_pathname, *canonical_pathname;
>        int scratch_chan;
>        struct target_section *sections = NULL, *sections_end = NULL;
>        char **matching;
>  
> -      scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, filename,
> -		   write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
> -			    &scratch_pathname);
> -#if defined(__GO32__) || defined(_WIN32) || defined(__CYGWIN__)
> -      if (scratch_chan < 0)
> +      if (is_target_filename (filename))
> +	{
> +	  if (target_filesystem_is_local)
> +	    filename += strlen (TARGET_SYSROOT_PREFIX);
> +	  else
> +	    load_via_target = 1;
> +	}
> +
> +      if (load_via_target)
>  	{
> -	  char *exename = alloca (strlen (filename) + 5);
> +	  if (write_files)
> +	    warning (_("writing into executable files is "
> +		       "not supported for %s sysroots"),
> +		     TARGET_SYSROOT_PREFIX);
> +
> +	  scratch_pathname = xstrdup (filename);
> +	  make_cleanup (xfree, scratch_pathname);
>  
> -	  strcat (strcpy (exename, filename), ".exe");
> -	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST, exename,
> -	     write_files ? O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
> -	     &scratch_pathname);
> +	  scratch_chan = -1;
> +
> +	  canonical_pathname = scratch_pathname;
>  	}
> +      else
> +	{
> +
> +	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST,
> +				filename, write_files ?
> +				O_RDWR | O_BINARY : O_RDONLY | O_BINARY,
> +				&scratch_pathname);
> +#if defined(__GO32__) || defined(_WIN32) || defined(__CYGWIN__)
> +	  if (scratch_chan < 0)
> +	    {
> +	      char *exename = alloca (strlen (filename) + 5);
> +
> +	      strcat (strcpy (exename, filename), ".exe");
> +	      scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST,
> +				    exename, write_files ?
> +				    O_RDWR | O_BINARY
> +				    : O_RDONLY | O_BINARY,
> +				    &scratch_pathname);
> +	    }
>  #endif
> -      if (scratch_chan < 0)
> -	perror_with_name (filename);
> +	  if (scratch_chan < 0)
> +	    perror_with_name (filename);
>  
> -      make_cleanup (xfree, scratch_pathname);
> +	  make_cleanup (xfree, scratch_pathname);
>  
> -      /* gdb_bfd_open (and its variants) prefers canonicalized pathname for
> -	 better BFD caching.  */
> -      canonical_pathname = gdb_realpath (scratch_pathname);
> -      make_cleanup (xfree, canonical_pathname);
> +	  /* gdb_bfd_open (and its variants) prefers canonicalized
> +	     pathname for better BFD caching.  */
> +	  canonical_pathname = gdb_realpath (scratch_pathname);
> +	  make_cleanup (xfree, canonical_pathname);
> +	}
>  
> -      if (write_files)
> +      if (write_files && !load_via_target)
>  	exec_bfd = gdb_bfd_fopen (canonical_pathname, gnutarget,
>  				  FOPEN_RUB, scratch_chan);
>        else
> @@ -218,7 +248,10 @@ exec_file_attach (const char *filename, int from_tty)
>  	}
>  
>        gdb_assert (exec_filename == NULL);
> -      exec_filename = gdb_realpath_keepfile (scratch_pathname);
> +      if (load_via_target)
> +	exec_filename = xstrdup (bfd_get_filename (exec_bfd));
> +      else
> +	exec_filename = gdb_realpath_keepfile (scratch_pathname);
>  
>        if (!bfd_check_format_matches (exec_bfd, bfd_object, &matching))
>  	{
> 

Re: [PATCH 7/9] Update exec_file_attach to cope with "target:" filenames

[Gary Benson]

Pedro Alves wrote:
> On 03/20/2015 04:48 PM, Gary Benson wrote:
> > This commit adds support for filenames prefixed	with "target:" to
> > exec_file_attach.  This is required to correctly follow inferior
> > exec* calls when a gdb_sysroot prefixed with "target:" is set.
> 
> Hmm, I don't see how.  Isn't this only true when target_pid_to_exec_file
> prepends the sysroot, which it doesn't yet?  I think this should move
> to that other series.

Search for gdb_sysroot in infrun.c.

> A couple bits could use more explanation (in commit log and/or
> comments):
> 
>  - Why is writing into executable files not supported with "target:" ?
>  - The skipping of gdb_realpath_keepfile.

Are these two additions ok?

diff --git a/gdb/exec.c b/gdb/exec.c
index 6b6fc7d..ce61303 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -192,6 +192,7 @@ exec_file_attach (const char *filename, int from_tty)
 
       if (load_via_target)
 	{
+	  /* gdb_bfd_fopen does not support "target:" filenames.  */
 	  if (write_files)
 	    warning (_("writing into executable files is "
 		       "not supported for %s sysroots"),
@@ -247,8 +248,10 @@ exec_file_attach (const char *filename, int from_tty)
 		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
 	}
 
+      /* gdb_realpath_keepfile resolves symlinks on the local
+	 filesystem and so cannot be used for "target:" files.  */
       gdb_assert (exec_filename == NULL);
       if (load_via_target)
 	exec_filename = xstrdup (bfd_get_filename (exec_bfd));
       else
 	exec_filename = gdb_realpath_keepfile (scratch_pathname);

Cheers,
Gary

--
http://gbenson.net/

Re: [PATCH 7/9] Update exec_file_attach to cope with "target:" filenames

[Pedro Alves]

On 04/01/2015 02:55 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 03/20/2015 04:48 PM, Gary Benson wrote:
>>> This commit adds support for filenames prefixed	with "target:" to
>>> exec_file_attach.  This is required to correctly follow inferior
>>> exec* calls when a gdb_sysroot prefixed with "target:" is set.
>>
>> Hmm, I don't see how.  Isn't this only true when target_pid_to_exec_file
>> prepends the sysroot, which it doesn't yet?  I think this should move
>> to that other series.
> 
> Search for gdb_sysroot in infrun.c.

Ah, thanks.  I went from memory (which obviously failed), while
I should have looked.

> 
>> A couple bits could use more explanation (in commit log and/or
>> comments):
>>
>>  - Why is writing into executable files not supported with "target:" ?
>>  - The skipping of gdb_realpath_keepfile.
> 
> Are these two additions ok?

Yes, thanks.

BTW, looking again at the patch I noticed a spurious new line:

> +	{
> +
> +	  scratch_chan = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST,

Patch is OK with those fixed.

> 
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 6b6fc7d..ce61303 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -192,6 +192,7 @@ exec_file_attach (const char *filename, int from_tty)
>  
>        if (load_via_target)
>  	{
> +	  /* gdb_bfd_fopen does not support "target:" filenames.  */
>  	  if (write_files)
>  	    warning (_("writing into executable files is "
>  		       "not supported for %s sysroots"),

That's a limitation that can be lifted later, by either
teaching gdb_bfd_fopen about "target:" or using an alternative
like gdb_bfd_open instead, right?

Thanks,
Pedro Alves

Re: [PATCH 7/9] Update exec_file_attach to cope with "target:" filenames

[Gary Benson]

Pedro Alves wrote:
> On 04/01/2015 02:55 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > On 03/20/2015 04:48 PM, Gary Benson wrote:
> > > > This commit adds support for filenames prefixed	with "target:" to
> > > > exec_file_attach.  This is required to correctly follow inferior
> > > > exec* calls when a gdb_sysroot prefixed with "target:" is set.
> > >
> > > Hmm, I don't see how.  Isn't this only true when target_pid_to_exec_file
> > > prepends the sysroot, which it doesn't yet?  I think this should move
> > > to that other series.
> > 
> > Search for gdb_sysroot in infrun.c.
> 
> Ah, thanks.  I went from memory (which obviously failed), while
> I should have looked.

I may make a followup patch to convert that bit to use exec_file_find
from my "Don't require 'file' for remote targets" series when that's
in.

> > diff --git a/gdb/exec.c b/gdb/exec.c
> > index 6b6fc7d..ce61303 100644
> > --- a/gdb/exec.c
> > +++ b/gdb/exec.c
> > @@ -192,6 +192,7 @@ exec_file_attach (const char *filename, int from_tty)
> >  
> >        if (load_via_target)
> >  	{
> > +	  /* gdb_bfd_fopen does not support "target:" filenames.  */
> >  	  if (write_files)
> >  	    warning (_("writing into executable files is "
> >  		       "not supported for %s sysroots"),
> 
> That's a limitation that can be lifted later, by either teaching
> gdb_bfd_fopen about "target:" or using an alternative like
> gdb_bfd_open instead, right?

Yes, it's not a permanent restriction, just stuff that's not written
yet.  The appropriate methods are in RSP I think, it's just gdb_bfd.c
that needs touching.  (It could probably be refactored some too,
maybe make gdb_bfd_open a special case of gdb_bfd_fopen and put all
the real code in the latter.) 

Cheers,
Gary

-- 
http://gbenson.net/

[PATCH 2/9] Introduce target_filesystem_is_local

[Gary Benson]

This commit introduces a new target method target_filesystem_is_local
which can be used to determine whether or not the filesystem accessed
by the target_fileio_* methods is the local filesystem.

gdb/ChangeLog:

	* target.h (struct target_ops) <to_filesystem_is_local>:
	New field.
	(target_filesystem_is_local): New definition.
	* target-delegates.c: Regenerate.
	* remote.c (remote_filesystem_is_local): New function.
	(init_remote_ops): Initialize to_filesystem_is_local.
---
 gdb/ChangeLog          |    9 +++++++++
 gdb/remote.c           |   10 ++++++++++
 gdb/target-delegates.c |   31 +++++++++++++++++++++++++++++++
 gdb/target.h           |   11 +++++++++++
 4 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 9c6d2e5..c0fab92 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -9890,6 +9890,15 @@ remote_hostio_send_command (int command_bytes, int which_packet,
   return ret;
 }
 
+/* Return nonzero if the filesystem accessed by the target_fileio_*
+   methods is the local filesystem, zero otherwise.  */
+
+static int
+remote_filesystem_is_local (struct target_ops *self)
+{
+  return 0;
+}
+
 /* Open FILENAME on the remote target, using FLAGS and MODE.  Return a
    remote file descriptor, or -1 if an error occurs (and set
    *REMOTE_ERRNO).  */
@@ -11841,6 +11850,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_supports_multi_process = remote_supports_multi_process;
   remote_ops.to_supports_disable_randomization
     = remote_supports_disable_randomization;
+  remote_ops.to_filesystem_is_local = remote_filesystem_is_local;
   remote_ops.to_fileio_open = remote_hostio_open;
   remote_ops.to_fileio_pwrite = remote_hostio_pwrite;
   remote_ops.to_fileio_pread = remote_hostio_pread;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 0c1309a..20ad257 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -2345,6 +2345,33 @@ debug_thread_address_space (struct target_ops *self, ptid_t arg1)
   return result;
 }
 
+static int
+delegate_filesystem_is_local (struct target_ops *self)
+{
+  self = self->beneath;
+  return self->to_filesystem_is_local (self);
+}
+
+static int
+tdefault_filesystem_is_local (struct target_ops *self)
+{
+  return 1;
+}
+
+static int
+debug_filesystem_is_local (struct target_ops *self)
+{
+  int result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_filesystem_is_local (...)\n", debug_target.to_shortname);
+  result = debug_target.to_filesystem_is_local (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_filesystem_is_local (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_int (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
 static void
 delegate_trace_init (struct target_ops *self)
 {
@@ -4024,6 +4051,8 @@ install_delegators (struct target_ops *ops)
     ops->to_thread_architecture = delegate_thread_architecture;
   if (ops->to_thread_address_space == NULL)
     ops->to_thread_address_space = delegate_thread_address_space;
+  if (ops->to_filesystem_is_local == NULL)
+    ops->to_filesystem_is_local = delegate_filesystem_is_local;
   if (ops->to_trace_init == NULL)
     ops->to_trace_init = delegate_trace_init;
   if (ops->to_download_tracepoint == NULL)
@@ -4227,6 +4256,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_can_run_breakpoint_commands = tdefault_can_run_breakpoint_commands;
   ops->to_thread_architecture = default_thread_architecture;
   ops->to_thread_address_space = default_thread_address_space;
+  ops->to_filesystem_is_local = tdefault_filesystem_is_local;
   ops->to_trace_init = tdefault_trace_init;
   ops->to_download_tracepoint = tdefault_download_tracepoint;
   ops->to_can_download_tracepoint = tdefault_can_download_tracepoint;
@@ -4374,6 +4404,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_can_run_breakpoint_commands = debug_can_run_breakpoint_commands;
   ops->to_thread_architecture = debug_thread_architecture;
   ops->to_thread_address_space = debug_thread_address_space;
+  ops->to_filesystem_is_local = debug_filesystem_is_local;
   ops->to_trace_init = debug_trace_init;
   ops->to_download_tracepoint = debug_download_tracepoint;
   ops->to_can_download_tracepoint = debug_can_download_tracepoint;
diff --git a/gdb/target.h b/gdb/target.h
index d2bd152..925abeb 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -825,6 +825,12 @@ struct target_ops
 
     /* Target file operations.  */
 
+    /* Return nonzero if the filesystem accessed by the
+       target_fileio_* methods is the local filesystem,
+       zero otherwise.  */
+    int (*to_filesystem_is_local) (struct target_ops *)
+      TARGET_DEFAULT_RETURN (1);
+
     /* Open FILENAME on the target, using FLAGS and MODE.  Return a
        target file descriptor, or -1 if an error occurs (and set
        *TARGET_ERRNO).  */
@@ -1920,6 +1926,11 @@ extern int target_search_memory (CORE_ADDR start_addr,
 
 /* Target file operations.  */
 
+/* Nonzero if the filesystem accessed by the target_fileio_*
+   methods is the local filesystem, zero otherwise.  */
+#define target_filesystem_is_local \
+  current_target.to_filesystem_is_local (&current_target)
+
 /* Open FILENAME on the target, using FLAGS and MODE.  Return a
    target file descriptor, or -1 if an error occurs (and set
    *TARGET_ERRNO).  */
-- 
1.7.1

Re: [PATCH 2/9] Introduce target_filesystem_is_local

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:

> This commit introduces a new target method target_filesystem_is_local
> which can be used to determine whether or not the filesystem accessed
> by the target_fileio_* methods is the local filesystem.
> 

Not sure about the default to true, but we can always flip that
around easily.

> +/* Nonzero if the filesystem accessed by the target_fileio_*
> +   methods is the local filesystem, zero otherwise.  */
> +#define target_filesystem_is_local \
> +  current_target.to_filesystem_is_local (&current_target)
> +

Please make this a function-like macro:

#define target_filesystem_is_local()

OK with that change.

Thanks,
Pedro Alves

[PATCH 5/9] Rearrange symfile_bfd_open

[Gary Benson]

symfile_bfd_open handles what were remote files as a special case.
Now that remote files are replaced with target the reverse is true
and the BFD opening, format checking and error printing code is
essentially duplicated.  This commit rearranges symfile_bfd_open
to treat local files as a special case, removing the duplication.

gdb/ChangeLog:

	* symfile.c (symfile_bfd_open): Reorder to remove duplicated
	checks and error messages.
---
 gdb/ChangeLog |    5 ++++
 gdb/symfile.c |   69 ++++++++++++++++++++++++--------------------------------
 2 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0d8dae7..0c35ffa 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1718,60 +1718,51 @@ set_initial_language (void)
    absolute).  In case of trouble, error() is called.  */
 
 bfd *
-symfile_bfd_open (const char *cname)
+symfile_bfd_open (const char *name)
 {
   bfd *sym_bfd;
-  int desc;
-  char *name, *absolute_name;
-  struct cleanup *back_to;
+  int desc = -1;
+  struct cleanup *back_to = make_cleanup (null_cleanup, 0);
 
-  if (is_target_filename (cname))
+  if (!is_target_filename (name))
     {
-      sym_bfd = gdb_bfd_open (cname, gnutarget, -1);
-      if (!sym_bfd)
-	error (_("`%s': can't open to read symbols: %s."), cname,
-	       bfd_errmsg (bfd_get_error ()));
-
-      if (!bfd_check_format (sym_bfd, bfd_object))
-	{
-	  make_cleanup_bfd_unref (sym_bfd);
-	  error (_("`%s': can't read symbols: %s."), cname,
-		 bfd_errmsg (bfd_get_error ()));
-	}
+      char *expanded_name, *absolute_name;
 
-      return sym_bfd;
-    }
-
-  name = tilde_expand (cname);	/* Returns 1st new malloc'd copy.  */
+      expanded_name = tilde_expand (name); /* Returns 1st new malloc'd copy.  */
 
-  /* Look down path for it, allocate 2nd new malloc'd copy.  */
-  desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, name,
-		O_RDONLY | O_BINARY, &absolute_name);
+      /* Look down path for it, allocate 2nd new malloc'd copy.  */
+      desc = openp (getenv ("PATH"),
+		    OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
+		    expanded_name, O_RDONLY | O_BINARY, &absolute_name);
 #if defined(__GO32__) || defined(_WIN32) || defined (__CYGWIN__)
-  if (desc < 0)
-    {
-      char *exename = alloca (strlen (name) + 5);
+      if (desc < 0)
+	{
+	  char *exename = alloca (strlen (expanded_name) + 5);
 
-      strcat (strcpy (exename, name), ".exe");
-      desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
-		    exename, O_RDONLY | O_BINARY, &absolute_name);
-    }
+	  strcat (strcpy (exename, expanded_name), ".exe");
+	  desc = openp (getenv ("PATH"),
+			OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
+			exename, O_RDONLY | O_BINARY, &absolute_name);
+	}
 #endif
-  if (desc < 0)
-    {
-      make_cleanup (xfree, name);
-      perror_with_name (name);
-    }
+      if (desc < 0)
+	{
+	  make_cleanup (xfree, expanded_name);
+	  perror_with_name (expanded_name);
+	}
 
-  xfree (name);
-  name = absolute_name;
-  back_to = make_cleanup (xfree, name);
+      xfree (expanded_name);
+      make_cleanup (xfree, absolute_name);
+      name = absolute_name;
+    }
 
   sym_bfd = gdb_bfd_open (name, gnutarget, desc);
   if (!sym_bfd)
     error (_("`%s': can't open to read symbols: %s."), name,
 	   bfd_errmsg (bfd_get_error ()));
-  bfd_set_cacheable (sym_bfd, 1);
+
+  if (!gdb_bfd_has_target_filename (sym_bfd))
+    bfd_set_cacheable (sym_bfd, 1);
 
   if (!bfd_check_format (sym_bfd, bfd_object))
     {
-- 
1.7.1

Re: [PATCH 5/9] Rearrange symfile_bfd_open

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:
> symfile_bfd_open handles what were remote files as a special case.
> Now that remote files are replaced with target the reverse is true
> and the BFD opening, format checking and error printing code is
> essentially duplicated.  This commit rearranges symfile_bfd_open
> to treat local files as a special case, removing the duplication.

Where were they already done?  Can you add a comment?

> 
> gdb/ChangeLog:
> 
> 	* symfile.c (symfile_bfd_open): Reorder to remove duplicated
> 	checks and error messages.
> ---
>  gdb/ChangeLog |    5 ++++
>  gdb/symfile.c |   69 ++++++++++++++++++++++++--------------------------------
>  2 files changed, 35 insertions(+), 39 deletions(-)
> 
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 0d8dae7..0c35ffa 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1718,60 +1718,51 @@ set_initial_language (void)
>     absolute).  In case of trouble, error() is called.  */
>  
>  bfd *
> -symfile_bfd_open (const char *cname)
> +symfile_bfd_open (const char *name)
>  {
>    bfd *sym_bfd;
> -  int desc;
> -  char *name, *absolute_name;
> -  struct cleanup *back_to;
> +  int desc = -1;
> +  struct cleanup *back_to = make_cleanup (null_cleanup, 0);
>  
> -  if (is_target_filename (cname))
> +  if (!is_target_filename (name))
>      {
> -      sym_bfd = gdb_bfd_open (cname, gnutarget, -1);
> -      if (!sym_bfd)
> -	error (_("`%s': can't open to read symbols: %s."), cname,
> -	       bfd_errmsg (bfd_get_error ()));
> -
> -      if (!bfd_check_format (sym_bfd, bfd_object))
> -	{
> -	  make_cleanup_bfd_unref (sym_bfd);
> -	  error (_("`%s': can't read symbols: %s."), cname,
> -		 bfd_errmsg (bfd_get_error ()));
> -	}
> +      char *expanded_name, *absolute_name;
>  
> -      return sym_bfd;
> -    }
> -
> -  name = tilde_expand (cname);	/* Returns 1st new malloc'd copy.  */
> +      expanded_name = tilde_expand (name); /* Returns 1st new malloc'd copy.  */
>  
> -  /* Look down path for it, allocate 2nd new malloc'd copy.  */
> -  desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH, name,
> -		O_RDONLY | O_BINARY, &absolute_name);
> +      /* Look down path for it, allocate 2nd new malloc'd copy.  */
> +      desc = openp (getenv ("PATH"),
> +		    OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
> +		    expanded_name, O_RDONLY | O_BINARY, &absolute_name);
>  #if defined(__GO32__) || defined(_WIN32) || defined (__CYGWIN__)
> -  if (desc < 0)
> -    {
> -      char *exename = alloca (strlen (name) + 5);
> +      if (desc < 0)
> +	{
> +	  char *exename = alloca (strlen (expanded_name) + 5);
>  
> -      strcat (strcpy (exename, name), ".exe");
> -      desc = openp (getenv ("PATH"), OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
> -		    exename, O_RDONLY | O_BINARY, &absolute_name);
> -    }
> +	  strcat (strcpy (exename, expanded_name), ".exe");
> +	  desc = openp (getenv ("PATH"),
> +			OPF_TRY_CWD_FIRST | OPF_RETURN_REALPATH,
> +			exename, O_RDONLY | O_BINARY, &absolute_name);
> +	}
>  #endif
> -  if (desc < 0)
> -    {
> -      make_cleanup (xfree, name);
> -      perror_with_name (name);
> -    }
> +      if (desc < 0)
> +	{
> +	  make_cleanup (xfree, expanded_name);
> +	  perror_with_name (expanded_name);
> +	}
>  
> -  xfree (name);
> -  name = absolute_name;
> -  back_to = make_cleanup (xfree, name);
> +      xfree (expanded_name);
> +      make_cleanup (xfree, absolute_name);
> +      name = absolute_name;
> +    }
>  
>    sym_bfd = gdb_bfd_open (name, gnutarget, desc);
>    if (!sym_bfd)
>      error (_("`%s': can't open to read symbols: %s."), name,
>  	   bfd_errmsg (bfd_get_error ()));
> -  bfd_set_cacheable (sym_bfd, 1);
> +
> +  if (!gdb_bfd_has_target_filename (sym_bfd))
> +    bfd_set_cacheable (sym_bfd, 1);
>  
>    if (!bfd_check_format (sym_bfd, bfd_object))
>      {
> 


Thanks,
Pedro Alves

Re: [PATCH 5/9] Rearrange symfile_bfd_open

[Gary Benson]

Pedro Alves wrote:
> On 03/20/2015 04:48 PM, Gary Benson wrote:
> > symfile_bfd_open handles what were remote files as a special case.
> > Now that remote files are replaced with target the reverse is true
> > and the BFD opening, format checking and error printing code is
> > essentially duplicated.  This commit rearranges symfile_bfd_open
> > to treat local files as a special case, removing the duplication.
> 
> Where were they already done?  Can you add a comment?

They were done twice in the same function:

  if remote:
    open bfd, check format, etc
    return
  local-specific stuff
  open bfd, check format, etc
  return

I rearranged it to be like this:

  if local:
      local-specific stuff
  open bfd, check format, etc
  return

How about I change the commit message to this:

  symfile_bfd_open handled what were remote files as a special case.
  Converting from "remote:" files to "target:" means the two cases
  are now very similar and may be merged.  This commit rearranges
  symfile_bfd_open to remove the duplicated code.

?

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH 5/9] Rearrange symfile_bfd_open

[Pedro Alves]

On 04/01/2015 04:50 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 03/20/2015 04:48 PM, Gary Benson wrote:
>>> symfile_bfd_open handles what were remote files as a special case.
>>> Now that remote files are replaced with target the reverse is true
>>> and the BFD opening, format checking and error printing code is
>>> essentially duplicated.  This commit rearranges symfile_bfd_open
>>> to treat local files as a special case, removing the duplication.
>>
>> Where were they already done?  Can you add a comment?
> 
> They were done twice in the same function:
> 
>   if remote:
>     open bfd, check format, etc
>     return
>   local-specific stuff
>   open bfd, check format, etc
>   return
> 
> I rearranged it to be like this:
> 
>   if local:
>       local-specific stuff
>   open bfd, check format, etc
>   return
> 
> How about I change the commit message to this:
> 
>   symfile_bfd_open handled what were remote files as a special case.
>   Converting from "remote:" files to "target:" means the two cases
>   are now very similar and may be merged.  This commit rearranges
>   symfile_bfd_open to remove the duplicated code.

Ah, I see now.  The pseudo code above helped me a lot too.  I'd
suggest putting in the commit log too.

Patch is OK.

Thanks,
Pedro Alves

[PATCH 3/9] Make gdb_bfd_open able to open BFDs using target fileio

[Gary Benson]

This commit updates gdb_bfd_open to access files using target
fileio functions if the supplied path starts with "target:"
and if the local and target filesystems are not the same.
This allows users to specify "set sysroot target:" and have
GDB access files locally or from the remote as appropriate.

The new functions in gdb_bfd.c are copies of functions from
remote.c. This duplication is intentional and will be removed
by the next commit in this series.

gdb/ChangeLog:

	* gdb/gdb_bfd.h (TARGET_SYSROOT_PREFIX): New definition.
	(is_target_filename): New declaration.
	(gdb_bfd_has_target_filename): Likewise.
	(gdb_bfd_open): Update documentation comment.
	* gdb_bfd.c (target.h): New include.
	(gdb/fileio.h): Likewise.
	(is_target_filename): New function.
	(gdb_bfd_has_target_filename): Likewise.
	(fileio_errno_to_host): Likewise.
	(gdb_bfd_iovec_fileio_open): Likewise.
	(gdb_bfd_iovec_fileio_pread): Likewise.
	(gdb_bfd_iovec_fileio_close): Likewise.
	(gdb_bfd_iovec_fileio_fstat): Likewise.
	(gdb_bfd_open): Use target fileio to access paths prefixed
	with "target:" where necessary.
---
 gdb/ChangeLog |   18 +++++
 gdb/gdb_bfd.c |  203 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdb_bfd.h |   29 +++++++-
 3 files changed, 246 insertions(+), 4 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 7543dae..43bd328 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -33,6 +33,8 @@
 #define MAP_FAILED ((void *) -1)
 #endif
 #endif
+#include "target.h"
+#include "gdb/fileio.h"
 
 typedef bfd *bfdp;
 DEF_VEC_P (bfdp);
@@ -141,6 +143,177 @@ eq_bfd (const void *a, const void *b)
 
 /* See gdb_bfd.h.  */
 
+int
+is_target_filename (const char *name)
+{
+  return startswith (name, TARGET_SYSROOT_PREFIX);
+}
+
+/* See gdb_bfd.h.  */
+
+int
+gdb_bfd_has_target_filename (struct bfd *abfd)
+{
+  return is_target_filename (bfd_get_filename (abfd));
+}
+
+
+/* Return the system error number corresponding to ERRNUM.  */
+
+static int
+fileio_errno_to_host (int errnum)
+{
+  switch (errnum)
+    {
+      case FILEIO_EPERM:
+        return EPERM;
+      case FILEIO_ENOENT:
+        return ENOENT;
+      case FILEIO_EINTR:
+        return EINTR;
+      case FILEIO_EIO:
+        return EIO;
+      case FILEIO_EBADF:
+        return EBADF;
+      case FILEIO_EACCES:
+        return EACCES;
+      case FILEIO_EFAULT:
+        return EFAULT;
+      case FILEIO_EBUSY:
+        return EBUSY;
+      case FILEIO_EEXIST:
+        return EEXIST;
+      case FILEIO_ENODEV:
+        return ENODEV;
+      case FILEIO_ENOTDIR:
+        return ENOTDIR;
+      case FILEIO_EISDIR:
+        return EISDIR;
+      case FILEIO_EINVAL:
+        return EINVAL;
+      case FILEIO_ENFILE:
+        return ENFILE;
+      case FILEIO_EMFILE:
+        return EMFILE;
+      case FILEIO_EFBIG:
+        return EFBIG;
+      case FILEIO_ENOSPC:
+        return ENOSPC;
+      case FILEIO_ESPIPE:
+        return ESPIPE;
+      case FILEIO_EROFS:
+        return EROFS;
+      case FILEIO_ENOSYS:
+        return ENOSYS;
+      case FILEIO_ENAMETOOLONG:
+        return ENAMETOOLONG;
+    }
+  return -1;
+}
+
+/* Wrapper for target_fileio_open suitable for passing as the
+   OPEN_FUNC argument to gdb_bfd_openr_iovec.  The supplied
+   OPEN_CLOSURE is unused.  */
+
+static void *
+gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *open_closure)
+{
+  const char *filename = bfd_get_filename (abfd);
+  int fd, target_errno;
+  int *stream;
+
+  gdb_assert (is_target_filename (filename));
+
+  fd = target_fileio_open (filename + strlen (TARGET_SYSROOT_PREFIX),
+			   FILEIO_O_RDONLY, 0,
+			   &target_errno);
+  if (fd == -1)
+    {
+      errno = fileio_errno_to_host (target_errno);
+      bfd_set_error (bfd_error_system_call);
+      return NULL;
+    }
+
+  stream = XCNEW (int);
+  *stream = fd;
+  return stream;
+}
+
+/* Wrapper for target_fileio_pread suitable for passing as the
+   PREAD_FUNC argument to gdb_bfd_openr_iovec.  */
+
+static file_ptr
+gdb_bfd_iovec_fileio_pread (struct bfd *abfd, void *stream, void *buf,
+			    file_ptr nbytes, file_ptr offset)
+{
+  int fd = *(int *) stream;
+  int target_errno;
+  file_ptr pos, bytes;
+
+  pos = 0;
+  while (nbytes > pos)
+    {
+      bytes = target_fileio_pread (fd, (gdb_byte *) buf + pos,
+				   nbytes - pos, offset + pos,
+				   &target_errno);
+      if (bytes == 0)
+        /* Success, but no bytes, means end-of-file.  */
+        break;
+      if (bytes == -1)
+	{
+	  errno = fileio_errno_to_host (target_errno);
+	  bfd_set_error (bfd_error_system_call);
+	  return -1;
+	}
+
+      pos += bytes;
+    }
+
+  return pos;
+}
+
+/* Wrapper for target_fileio_close suitable for passing as the
+   CLOSE_FUNC argument to gdb_bfd_openr_iovec.  */
+
+static int
+gdb_bfd_iovec_fileio_close (struct bfd *abfd, void *stream)
+{
+  int fd = *(int *) stream;
+  int target_errno;
+
+  xfree (stream);
+
+  /* Ignore errors on close.  These may happen with remote
+     targets if the connection has already been torn down.  */
+  target_fileio_close (fd, &target_errno);
+
+  /* Zero means success.  */
+  return 0;
+}
+
+/* Wrapper for target_fileio_fstat suitable for passing as the
+   STAT_FUNC argument to gdb_bfd_openr_iovec.  */
+
+static int
+gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream,
+			    struct stat *sb)
+{
+  int fd = *(int *) stream;
+  int target_errno;
+  int result;
+
+  result = target_fileio_fstat (fd, sb, &target_errno);
+  if (result == -1)
+    {
+      errno = fileio_errno_to_host (target_errno);
+      bfd_set_error (bfd_error_system_call);
+    }
+
+  return result;
+}
+
+/* See gdb_bfd.h.  */
+
 struct bfd *
 gdb_bfd_open (const char *name, const char *target, int fd)
 {
@@ -150,6 +323,36 @@ gdb_bfd_open (const char *name, const char *target, int fd)
   struct gdb_bfd_cache_search search;
   struct stat st;
 
+  if (is_target_filename (name))
+    {
+      if (!target_filesystem_is_local)
+	{
+	  gdb_assert (fd == -1);
+
+	  abfd = gdb_bfd_openr_iovec (name, target,
+				      gdb_bfd_iovec_fileio_open, NULL,
+				      gdb_bfd_iovec_fileio_pread,
+				      gdb_bfd_iovec_fileio_close,
+				      gdb_bfd_iovec_fileio_fstat);
+
+	  if (abfd != NULL || errno != ENOSYS)
+	    return abfd;
+
+	  /* gdb_bfd_iovec_fileio_open failed with ENOSYS.  This can
+	     happen, for example, with vgdb (Valgrind GDB), which
+	     presents itself as a remote target but works on the local
+	     filesystem: it does not implement remote get and users
+	     are not expected to set gdb_sysroot.  To handle this case
+	     we fall back to trying the local filesystem iff
+	     gdb_sysroot is exactly TARGET_SYSROOT_PREFIX.  */
+	  if (gdb_sysroot == NULL
+	      || strcmp (gdb_sysroot, TARGET_SYSROOT_PREFIX) != 0)
+	    return NULL;
+	}
+
+      name += strlen (TARGET_SYSROOT_PREFIX);
+    }
+
   if (gdb_bfd_cache == NULL)
     gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
 				       xcalloc, xfree);
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 05b6870..8fbdf36 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -24,11 +24,32 @@
 
 DECLARE_REGISTRY (bfd);
 
+/* If supplied a path starting with this sequence, gdb_bfd_open will
+   open BFDs using target fileio operations.  */
+
+#define TARGET_SYSROOT_PREFIX "target:"
+
+/* Returns nonzero if NAME starts with TARGET_SYSROOT_PREFIX, zero
+   otherwise.  */
+
+int is_target_filename (const char *name);
+
+/* Returns nonzero if the filename associated with ABFD starts with
+   TARGET_SYSROOT_PREFIX, zero otherwise.  */
+
+int gdb_bfd_has_target_filename (struct bfd *abfd);
+
 /* Open a read-only (FOPEN_RB) BFD given arguments like bfd_fopen.
-   Returns NULL on error.  On success, returns a new reference to the
-   BFD, which must be freed with gdb_bfd_unref.  BFDs returned by this
-   call are shared among all callers opening the same file.  If FD is
-   not -1, then after this call it is owned by BFD.  */
+   If NAME starts with TARGET_SYSROOT_PREFIX then the BFD will be
+   opened using target fileio operations if necessary.  Returns NULL
+   on error.  On success, returns a new reference to the BFD, which
+   must be freed with gdb_bfd_unref.  BFDs returned by this call are
+   shared among all callers opening the same file.  If FD is not -1,
+   then after this call it is owned by BFD.  If the BFD was not
+   accessed using target fileio operations then the filename
+   associated with the BFD and accessible with bfd_get_filename will
+   not be exactly NAME but rather NAME with TARGET_SYSROOT_PREFIX
+   stripped.  */
 
 struct bfd *gdb_bfd_open (const char *name, const char *target, int fd);
 
-- 
1.7.1

Re: [PATCH 3/9] Make gdb_bfd_open able to open BFDs using target fileio

[Pedro Alves]

On 03/20/2015 04:48 PM, Gary Benson wrote:
> This commit updates gdb_bfd_open to access files using target
> fileio functions if the supplied path starts with "target:"
> and if the local and target filesystems are not the same.
> This allows users to specify "set sysroot target:" and have
> GDB access files locally or from the remote as appropriate.
> 
> The new functions in gdb_bfd.c are copies of functions from
> remote.c. This duplication is intentional and will be removed
> by the next commit in this series.

OK.

Thanks,
Pedro Alves

Re: [PATCH 0/9] New default sysroot "target:"

[Pedro Alves]

On 03/20/2015 04:47 PM, Gary Benson wrote:
> Hi all,
> 
> This series introduces a new "target:" prefix to "set sysroot".  Files
> specified with a "target:" prefix will be loaded via the target: from
> the remote if the target is remote, and from the local file system
> otherwise.  This new prefix replaces the "remote:" prefix, and the
> final patch in the series makes it the default.
> 
> The way the "target:" prefix is implemented differs somewhat from the
> way the "remote:" prefix was implemented:
> 
>  - It's hooked in at a lower level.  The remote stuff looked to have
>    been added piecemeal: various BFD-opening functions did checks on
>    their filenames and diverted to remote_bfd_open.  There was also
>    gdb_bfd_open_maybe_remote, which handled both local and remote
>    cases.  The "target:" prefix is baked into gdb_bfd_open, so all
>    functions that open BFDs gain support.
> 
>  - Various functions locally strip the "target:" prefix from the
>    filenames they're working on if the target filesystem is the
>    same as the local filesystem.  This serves two purposes:
> 
>      1) It ensures files accessed locally are handled the same
>         way regardless of how they are specified.  Things like
> 	the shared library search algorithm in solib_find, for
> 	example.
> 
>      2) It avoids cluttering GDB's output with "target:" prefixes.
> 
> Built and regtested on RHEL 6.6 x86_64.
> 
> Ok to commit?

Awesome work.

Please remember to update the LocalRemoteFeatureParity page once
this is in.

Thanks,
Pedro Alves

Re: [PATCH 0/9] New default sysroot "target:"

[Gary Benson]

Pedro Alves wrote:
> On 03/20/2015 04:47 PM, Gary Benson wrote:
> > This series introduces a new "target:" prefix to "set sysroot".
> > Files specified with a "target:" prefix will be loaded via the
> > target: from the remote if the target is remote, and from the
> > local file system otherwise.  This new prefix replaces the
> > "remote:" prefix, and the final patch in the series makes it
> > the default.
> > 
> > The way the "target:" prefix is implemented differs somewhat
> > from the way the "remote:" prefix was implemented:
> > 
> >  - It's hooked in at a lower level.  The remote stuff looked to
> >    have been added piecemeal: various BFD-opening functions did
> >    checks on their filenames and diverted to remote_bfd_open.
> >    There was also gdb_bfd_open_maybe_remote, which handled both
> >    local and remote cases.  The "target:" prefix is baked into
> >    gdb_bfd_open, so all functions that open BFDs gain support.
> > 
> >  - Various functions locally strip the "target:" prefix from the
> >    filenames they're working on if the target filesystem is the
> >    same as the local filesystem.  This serves two purposes:
> > 
> >      1) It ensures files accessed locally are handled the same
> >         way regardless of how they are specified.  Things like
> >         the shared library search algorithm in solib_find, for
> >         example.
> > 
> >      2) It avoids cluttering GDB's output with "target:"
> >         prefixes.
> > 
> > Built and regtested on RHEL 6.6 x86_64.
> > 
> > Ok to commit?
> 
> Awesome work.
> 
> Please remember to update the LocalRemoteFeatureParity page
> once this is in.

This is now pushed, thanks for the review.  I deleted the bit
about remote:->target: on the LocalRemoteFeatureParity page,
I hope that's correct.  I also made a comment on one of the
bugs (neither is ready to be closed yet).

Cheers,
Gary

-- 
http://gbenson.net/

[PING][PATCH 0/9] New default sysroot "target:"

[Gary Benson]

Ping:
https://sourceware.org/ml/gdb-patches/2015-03/msg00638.html

Gary Benson wrote:
> Hi all,
> 
> This series introduces a new "target:" prefix to "set sysroot".  Files
> specified with a "target:" prefix will be loaded via the target: from
> the remote if the target is remote, and from the local file system
> otherwise.  This new prefix replaces the "remote:" prefix, and the
> final patch in the series makes it the default.
> 
> The way the "target:" prefix is implemented differs somewhat from the
> way the "remote:" prefix was implemented:
> 
>  - It's hooked in at a lower level.  The remote stuff looked to have
>    been added piecemeal: various BFD-opening functions did checks on
>    their filenames and diverted to remote_bfd_open.  There was also
>    gdb_bfd_open_maybe_remote, which handled both local and remote
>    cases.  The "target:" prefix is baked into gdb_bfd_open, so all
>    functions that open BFDs gain support.
> 
>  - Various functions locally strip the "target:" prefix from the
>    filenames they're working on if the target filesystem is the
>    same as the local filesystem.  This serves two purposes:
> 
>      1) It ensures files accessed locally are handled the same
>         way regardless of how they are specified.  Things like
> 	the shared library search algorithm in solib_find, for
> 	example.
> 
>      2) It avoids cluttering GDB's output with "target:" prefixes.
> 
> Built and regtested on RHEL 6.6 x86_64.
> 
> Ok to commit?
> 
> Thanks,
> Gary
> 
> --
> http://gbenson.net/