[1/2] Warn when accessing binaries over RSP

  GDB provides no indicator of progress during file operations, and can
appear to have locked up during slow remote transfers.  This commit
updates GDB to print a warning each time a file is accessed over RSP.
An additional message detailing how to avoid remote transfers is
printed for the first transfer only.

gdb/ChangeLog:

	* gdb_bfd.c (gdb_bfd_iovec_fileio_open): Print warnings when
	BFDs are opened via the remote protocol.

gdb/testsuite/ChangeLog:

	* gdb.trace/pending.exp: Cope with remote transfer warnings.
---
 gdb/ChangeLog                       |    5 +++++
 gdb/gdb_bfd.c                       |   33 +++++++++++++++++++++++++++++----
 gdb/testsuite/ChangeLog             |    4 ++++
 gdb/testsuite/gdb.trace/pending.exp |    8 ++++----
 4 files changed, 42 insertions(+), 8 deletions(-)
  

* Gary Benson <gbenson@redhat.com> [2015-08-05 16:28:15 +0100]:

> 
> diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
> index 1781d80..b511777 100644
> --- a/gdb/gdb_bfd.c
> +++ b/gdb/gdb_bfd.c
> @@ -219,13 +219,38 @@ gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *inferior)
>    const char *filename = bfd_get_filename (abfd);
>    int fd, target_errno;
>    int *stream;
> +  struct target_ops *ops = find_target_at (process_stratum);
>  
>    gdb_assert (is_target_filename (filename));
> +  filename += strlen (TARGET_SYSROOT_PREFIX);
> +
> +  /* GDB provides no indicator of progress during file operations, and
> +     can appear to have locked up during slow remote transfers, so we
> +     inform the user what is happening and suggest a way out.  It's
> +     unpleasant that we need to detect remote targets this way (rather
> +     than putting the warnings in remote_hostio_open), but it's not
> +     possible for remote_hostio_open to differentiate between
> +     accessing inferior binaries (which can be bypassed) and accessing
> +     things like /proc/ (which is unavoidable).  */
> +  if (strcmp (ops->to_shortname, "remote") == 0
> +      || strcmp (ops->to_shortname, "extended-remote") == 0)
> +    {
> +      static int warning_issued = 0;
> +
> +      printf_unfiltered (_("Reading %s from remote target\n"),
> +			 filename);
> +
> +      if (!warning_issued)
> +	{
> +	  warning (_("File transfers from remote targets can be slow.\n"
> +		     "Use \"set sysroot\" to access files locally"
> +		     " instead."));
> +	  warning_issued = 1;
> +	}
> +    }

Altering the behaviour based on to_shortname feels like breaking this
nice target OO model we have.

Could the warning not be moved down into target_fileio_open instead?

Or if that's really not an appropriate place should we add a new
target method?

Thanks,
Andrew
  

Andrew Burgess wrote:
> * Gary Benson <gbenson@redhat.com> [2015-08-05 16:28:15 +0100]:
> > 
> > diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
> > index 1781d80..b511777 100644
> > --- a/gdb/gdb_bfd.c
> > +++ b/gdb/gdb_bfd.c
> > @@ -219,13 +219,38 @@ gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *inferior)
> >    const char *filename = bfd_get_filename (abfd);
> >    int fd, target_errno;
> >    int *stream;
> > +  struct target_ops *ops = find_target_at (process_stratum);
> >  
> >    gdb_assert (is_target_filename (filename));
> > +  filename += strlen (TARGET_SYSROOT_PREFIX);
> > +
> > +  /* GDB provides no indicator of progress during file operations, and
> > +     can appear to have locked up during slow remote transfers, so we
> > +     inform the user what is happening and suggest a way out.  It's
> > +     unpleasant that we need to detect remote targets this way (rather
> > +     than putting the warnings in remote_hostio_open), but it's not
> > +     possible for remote_hostio_open to differentiate between
> > +     accessing inferior binaries (which can be bypassed) and accessing
> > +     things like /proc/ (which is unavoidable).  */
> > +  if (strcmp (ops->to_shortname, "remote") == 0
> > +      || strcmp (ops->to_shortname, "extended-remote") == 0)
> > +    {
> > +      static int warning_issued = 0;
> > +
> > +      printf_unfiltered (_("Reading %s from remote target\n"),
> > +			 filename);
> > +
> > +      if (!warning_issued)
> > +	{
> > +	  warning (_("File transfers from remote targets can be slow.\n"
> > +		     "Use \"set sysroot\" to access files locally"
> > +		     " instead."));
> > +	  warning_issued = 1;
> > +	}
> > +    }
> 
> Altering the behaviour based on to_shortname feels like breaking
> this nice target OO model we have.

Yeah... :-/

> Could the warning not be moved down into target_fileio_open instead?

Not so much target_fileio_open as remote_hostio_open; only remote
targets need the warning.  And originally I thought no, the warning
couldn't go there, because target_fileio_open/remote_hostio_open is
used for various internal things such as /proc/ file reads on Linux
that the user shouldn't see.

...however...

remote_hostio_open *can* differentiate between reading inferior
binaries and reading internal stuff because the internal stuff is
accessed with the INF argument NULL and binaries are accessed with
a non-NULL INF.

So I can do that, if it doesn't seem too hacky.

> Or if that's really not an appropriate place should we add a new
> target method?

I considered that but couldn't think of a good name :-)
target_fileio_warn_if_slow ??
I can do that too.

Cheers,
Gary