[7/9] Update exec_file_attach to cope with "target:" filenames
Commit Message
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?
Cheers,
Gary
--
http://gbenson.net/
Comments
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
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
@@ -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);