Re: [PATCH] bfd: add new bfd_cache_size() function

[Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>]

On 2023-10-06 18:03, Andrew Burgess via Binutils wrote:
> In GDB we have a problem with the BFD cache.
> 
> As GDB runs for a potentially extended period of time, if the BFD
> cache holds a file descriptor for an open on-disk file, this can, on
> some targets (e.g. Win32) prevent the OS writing to the file.
> 
> This might, for example, prevent a user from recompiling their
> executable as GDB is (via the BFD cache) holding an open reference to
> that file.
> 
> Another problem, relates to bfd_stat, for BFDs that are using the BFD
> cache (i.e. they call cache_bstat to implement bfd_stat).  The
> cache_bstat function finds the BFD in the cache, opening the file if
> needed, and then uses fstat on the open file descriptor.
> 
> What this means is that, if the on-disk file changes, but the cache
> was holding an open reference to the file, the bfd_stat will return
> the 'struct stat' for the old file, not the new file.
> 
> Now, for this second problem, we might be tempted to make use of an
> actual stat call, instead of calling bfd_stat, however, this isn't
> ideal as we have some BFDs that use a custom iovec, and implement the
> various functions over GDB's remote protocol.  By using bfd_stat we
> can have a single call that should work for both local files, and for
> remote files.
> 
> To solve both of these problems GDB has calls to bfd_cache_close_all
> sprinkled around its code base.  And in theory this should work fine.
> 
> However, I recently ran into a case where we had missed a
> bfd_cache_close_all call, and as a result some BFDs were held open.
> This caused a bfd_stat call to return an unexpected result (old file
> vs new file).
> 
> What I'd like is some way within GDB that I can do:
> 
>    gdb_assert ( /* Nothing is held open in the cache.  */ );
> 
> As this would allow GDB to quickly identify when we've missed some
> bfd_cache_close_all calls.
> 
> And so, to support this, I would like to add a new bfd_cache_size
> function.  This function returns an integer, which is the number of
> open files in the cache.  I can then start adding:
> 
>    gdb_assert (bfd_cache_size() == 0);
> 
> to GDB in some strategic spots, and start fixing all of the missing
> bfd_cache_close_all calls that crop up as a result.

I think this sounds reasonable, but I'm not a maintainer so you need 
someone else to approve this.