Re: [PATCH] Mark object files with "target:" filenames as OBJF_NONLOCAL_FILENAME

[Doug Evans <dje at google dot com>]

On Tue, Apr 14, 2015 at 4:41 AM, Gary Benson <gbenson@redhat.com> wrote:
> Doug Evans wrote:
>> On Mon, Apr 13, 2015 at 12:07 PM, Gary Benson <gbenson@redhat.com> wrote:
>> > This patch introduces a new objfile flag OBJF_NONLOCAL_FILENAME to
>> > denote that objfile->original_name and objfile->obfd->filename are
>> > filenames referring to files on filesystems other than GDB's local
>> > filesystem.  allocate_objfile is updated to force
>> > OBJF_NONLOCAL_FILENAME if the specified name starts with "target:",
>> > and to not attempt to expand the name using gdb_abspath if flags has
>> > OBJF_NONLOCAL_FILENAME set.  load_auto_scripts_for_objfile is updated
>> > to not attempt loading of auto-load scripts for objfiles with
>> > OBJF_NONLOCAL_FILENAME set in their flags.
>> >
>> > A new flag was created rather than reusing OBJF_NOT_FILENAME because
>> > setting that flag would stop Python's gdb.lookup_objfile from seeing
>> > the file, and it's not clear that that's desirable.
>> >
>> > Without this patch you *sometimes* get things like:
>> >
>> >   Reading symbols from /home/gary/target:/lib64/libreadline.so.6...
>> >
>> > I haven't figured out why this doesn't always happen but it's plainly
>> > wrong :)
>> >
>> > Built and regtested on RHEL6.6 x86_64.
>> >
>> > Ok to commit?
>> >
>> > Cheers,
>> > Gary
>> >
>> > gdb/ChangeLog:
>> >
>> >         * objfiles.h (OBJF_NONLOCAL_FILENAME): New define.
>> >         * objfiles.c (allocate_objfile): Force OBJF_NONLOCAL_FILENAME
>> >         for BFDs with "target:" filenames.  Do not attempt to expand
>> >         name if flags has OBJF_NONLOCAL_FILENAME set.
>> >         * auto-load.c (load_auto_scripts_for_objfile): Do not attempt
>> >         to auto-load scripts for OBJF_NONLOCAL_FILENAME objfiles.
>>
>> My first thought is that we'll be recording something twice, and that
>> can lead to problems (e.g., effort has to be expended to keep them in
>> sync).
>> "is nonlocal" is already specified by the "target:" in the name.
>>
>> While I'm all for building on "foo:bar" in path names (target:foo,
>> remote:foo, and so on), IWBN to build a library on top of that
>> rather than have sideband tables that recorded such extra info.
>> [Down the road I can imagine having a class for such things such
>> that we could augment what's recorded beyond just a "foo:bar"
>> string, but that's later, if ever.]
>>
>> IOW, how about having an "is non-local" predicate that is invoked on
>> the path whenever needed?
>> [it could be the current "is_target_filename" or if you wanted to add
>> a layer of abstraction that might be ok, depending on how this might
>> evolve]
>>
>> I realize this is a bit incongruous with OBJF_NOT_FILENAME, but I'd
>> rather head in the above direction than adding more OBJF_ flags.
>>
>> Thoughts?
>
> I'm happy to remake this patch using "is_target_filename".  I'll do
> that and mail a version 2 tomorrow.
>
> (I've been thinking we might need something more than a prefix at some
> point, maybe something more URL-like, but like you say, we don't need
> that right now.)

I was thinking, and this is not well thought out,
maybe there's value in replacing OBJF_NOT_FILENAME with a flag
that says the string is "foo:bar", and then we could have another prefix
for files that are currently marked with OBJF_NOT_FILENAME.
Just food for thought, or not.