Re: [PATCH 0/3] bfd cache: tighten match criteria and debug commands.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* Pedro Alves <palves@redhat.com> [2015-04-13 22:17:34 +0100]:

> On 04/13/2015 06:30 PM, Andrew Burgess wrote:
> > The consensus in the thread was, I think, that using inode information
> > might be the best solution.  I did look at this, but was not sure the
> > right place to hook this in; I guess I'd need to add some new bfd
> > functions.
>
> The inode/dev info is available right there in the same fstat call
> you are hooking into to retrieve the file size:

<snip>

> That "st" is a "struct stat", which contains:
>
>            struct stat {
>                dev_t     st_dev;     /* ID of device containing file */
>                ino_t     st_ino;     /* inode number */
>                ...
>                off_t     st_size;    /* total size, in bytes */
>
> You'd just need to do the exact same you're already doing, but
> store st_dev/st_ino instead of st_size?  What am I missing?

It's just my inability to communicate properly, my uncertainty only
related to how I would implement a windows suitable solution, as was
discussed in the thread I linked previously.

I'd still need to add a bfd interface for fetching the inode though,
to replace this snippet:

> @@ -466,6 +476,7 @@ gdb_bfd_ref (struct bfd *abfd)
>    gdata = bfd_zalloc (abfd, sizeof (struct gdb_bfd_data));
>    gdata->refc = 1;
>    gdata->mtime = bfd_get_mtime (abfd);
> +  gdata->size = bfd_get_size (abfd);
>    gdata->archive_bfd = NULL;
>    bfd_usrdata (abfd) = gdata;

and this new bfd interface will need to work in conjunction with the
bfd cache mechanism, where the actual bfd could be closed; I don't
know all the details of that code so I'll have to go check it out...

> Do note that separate_debug_file_exists already does this.

Thanks, I'll take a look at that.

>
> > Given that any solution using inodes would probably be on top of the
> > existing match criteria, and that obtaining the file size is pretty
> > cheep, I figured there's little harm is having this in place now, and
> > adding an inode based criteria later, if wanted.
>
> Although my first reaction was that it does sounds useful for
> Windows which leaves st_ino==0, as described in separate_debug_file_exists,
> I think that we should instead not share the bfds even if the file
> size matches: it's not hard to rebuild the program/library you're debugging
> and end up with the same file size but different contents...  A caching/sharing
> system that works in most cases, but sometimes fails is not a good system.

OK, thanks for the feedback, I'll try to come up with an inode based
patch instead.

Andrew