Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Gary Benson <gbenson@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 7/9] Update exec_file_attach to cope with "target:" filenames
Date: Wed, 01 Apr 2015 12:14:00 -0000	[thread overview]
Message-ID: <551BE12D.1080300@redhat.com> (raw)
In-Reply-To: <1426870087-32654-8-git-send-email-gbenson@redhat.com>

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))
>  	{
> 


  reply	other threads:[~2015-04-01 12:14 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-20 16:48 [PATCH 0/9] New default sysroot "target:" Gary Benson
2015-03-20 16:48 ` [PATCH 4/9] Convert "remote:" sysroots to "target:" and remove "remote:" Gary Benson
2015-04-01 12:13   ` Pedro Alves
2015-04-01 13:48     ` Gary Benson
2015-03-20 16:48 ` [PATCH 1/9] Introduce target_fileio_fstat Gary Benson
2015-04-01 12:11   ` Pedro Alves
2015-03-20 16:48 ` [PATCH 6/9] Strip "target:" prefix in solib_find if accessing local files Gary Benson
2015-04-01 12:14   ` Pedro Alves
2015-03-20 16:57 ` [PATCH 9/9] Document "target:" sysroot changes Gary Benson
2015-03-20 17:51   ` Eli Zaretskii
2015-04-01 12:15   ` Pedro Alves
2015-03-20 16:57 ` [PATCH 8/9] Make the default sysroot be "target:" Gary Benson
2015-04-01 12:15   ` Pedro Alves
2015-04-01 14:12     ` Gary Benson
2015-04-01 14:18       ` Pedro Alves
2015-03-20 17:18 ` [PATCH 7/9] Update exec_file_attach to cope with "target:" filenames Gary Benson
2015-04-01 12:14   ` Pedro Alves [this message]
2015-04-01 13:55     ` Gary Benson
2015-04-01 14:16       ` Pedro Alves
2015-04-01 15:59         ` Gary Benson
2015-03-20 17:32 ` [PATCH 2/9] Introduce target_filesystem_is_local Gary Benson
2015-04-01 12:11   ` Pedro Alves
2015-03-20 17:46 ` [PATCH 5/9] Rearrange symfile_bfd_open Gary Benson
2015-04-01 12:13   ` Pedro Alves
2015-04-01 15:50     ` Gary Benson
2015-04-01 16:03       ` Pedro Alves
2015-03-20 17:47 ` [PATCH 3/9] Make gdb_bfd_open able to open BFDs using target fileio Gary Benson
2015-04-01 12:13   ` Pedro Alves
2015-04-01 12:17 ` [PATCH 0/9] New default sysroot "target:" Pedro Alves
2015-04-02 13:00   ` Gary Benson
2015-04-01 12:22 ` [PING][PATCH " Gary Benson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=551BE12D.1080300@redhat.com \
    --to=palves@redhat.com \
    --cc=gbenson@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox