| Message ID | 1428952063-2121-1-git-send-email-gbenson@redhat.com |
|---|---|
| State | Superseded |
| Headers |
Received: (qmail 91179 invoked by alias); 13 Apr 2015 19:07:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 91166 invoked by uid 89); 13 Apr 2015 19:07:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 13 Apr 2015 19:07:46 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3DJ7j6B008539 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for <gdb-patches@sourceware.org>; Mon, 13 Apr 2015 15:07:45 -0400 Received: from blade.nx (ovpn-116-95.ams2.redhat.com [10.36.116.95]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3DJ7iSQ024169 for <gdb-patches@sourceware.org>; Mon, 13 Apr 2015 15:07:44 -0400 Received: from blade.nx (localhost [127.0.0.1]) by blade.nx (Postfix) with ESMTP id 4C028263EEB for <gdb-patches@sourceware.org>; Mon, 13 Apr 2015 20:07:43 +0100 (BST) From: Gary Benson <gbenson@redhat.com> To: gdb-patches@sourceware.org Subject: [PATCH] Mark object files with "target:" filenames as OBJF_NONLOCAL_FILENAME Date: Mon, 13 Apr 2015 20:07:43 +0100 Message-Id: <1428952063-2121-1-git-send-email-gbenson@redhat.com> X-IsSubscribed: yes |
Commit Message
Gary Benson
April 13, 2015, 7:07 p.m. UTC
Hi all, 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. --- gdb/ChangeLog | 9 +++++++++ gdb/auto-load.c | 7 +++++-- gdb/objfiles.c | 6 +++++- gdb/objfiles.h | 5 +++++ 4 files changed, 24 insertions(+), 3 deletions(-)
Comments
On Mon, Apr 13, 2015 at 12:07 PM, Gary Benson <gbenson@redhat.com> wrote: > Hi all, > > 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?
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.) Cheers, Gary
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.
Doug Evans wrote: > On Tue, Apr 14, 2015 at 4:41 AM, Gary Benson <gbenson@redhat.com> wrote: > > Doug Evans wrote: > > > 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'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. Yeah, if we use some form of URL then the OBJF_NOT_FILENAME ones can fit in there too. Local files would be file://, target ones could be target:// maybe, or target: can be a magic prefix that gets expanded into whatever is necessary. The reason I was thinking we might need something more that what's there now, by the way, is that I was trying to figure out if loaded BFDs should be keyed to the inferior they came from. If we had two non-local inferiors both with target:/lib64/libc.so.6, would GDB consider those the same file somehow? I don't know. Cheers, Gary
On 04/14/2015 12:27 AM, Doug Evans wrote: > 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.] I agree. I like that -- I can definitely see us with something like a "struct gdb_path" object rather than a passing around a bare char * / string. > > 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 agree, for now is_target_filename seems good. > > 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. *nod* Thanks, Pedro Alves
Pedro Alves wrote: > On 04/14/2015 12:27 AM, Doug Evans wrote: > > 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 agree, for now is_target_filename seems good. I mailed a version 2 of this patch this morning, with that change: https://sourceware.org/ml/gdb-patches/2015-04/msg00547.html Cheers, Gary
diff --git a/gdb/auto-load.c b/gdb/auto-load.c index 778eeb6..6782f98 100644 --- a/gdb/auto-load.c +++ b/gdb/auto-load.c @@ -1195,8 +1195,11 @@ load_auto_scripts_for_objfile (struct objfile *objfile) { /* Return immediately if auto-loading has been globally disabled. This is to handle sequencing of operations during gdb startup. - Also return immediately if OBJFILE is not actually a file. */ - if (!global_auto_load || (objfile->flags & OBJF_NOT_FILENAME) != 0) + Also return immediately if OBJFILE is not a file on the local + filesystem. */ + if (!global_auto_load + || (objfile->flags & (OBJF_NOT_FILENAME + | OBJF_NONLOCAL_FILENAME)) != 0) return; /* Load any extension language scripts for this objfile. diff --git a/gdb/objfiles.c b/gdb/objfiles.c index ff20bc8..c1ad815 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -295,13 +295,17 @@ allocate_objfile (bfd *abfd, const char *name, int flags) objfile_alloc_data (objfile); + /* Target filenames are non-local. */ + if (name != NULL && is_target_filename (name)) + flags |= OBJF_NONLOCAL_FILENAME; + if (name == NULL) { gdb_assert (abfd == NULL); gdb_assert ((flags & OBJF_NOT_FILENAME) != 0); expanded_name = xstrdup ("<<anonymous objfile>>"); } - else if ((flags & OBJF_NOT_FILENAME) != 0) + else if ((flags & (OBJF_NOT_FILENAME | OBJF_NONLOCAL_FILENAME)) != 0) expanded_name = xstrdup (name); else expanded_name = gdb_abspath (name); diff --git a/gdb/objfiles.h b/gdb/objfiles.h index a0dc69b..fbc3277 100644 --- a/gdb/objfiles.h +++ b/gdb/objfiles.h @@ -459,6 +459,11 @@ struct objfile #define OBJF_NOT_FILENAME (1 << 6) +/* ORIGINAL_NAME and OBFD->FILENAME are filenames on non-local + filesystems and should be treated as opaque. */ + +#define OBJF_NONLOCAL_FILENAME (1 << 7) + /* Declarations for functions defined in objfiles.c */ extern struct objfile *allocate_objfile (bfd *, const char *name, int);