Re: [PATCH] Consolidate calls to bfd_set_cacheable

[John Baldwin <jhb@FreeBSD.org>]

On 8/4/23 9:19 AM, Tom Tromey wrote:
> I noticed that some spots in gdb call bfd_set_cacheable after opening
> a BFD.
> 
> The BFD file cache is a bit odd.  BFDs that are opened locally are
> unconditionally registered with the cache, and their underlying file
> descriptor will always be closed when bfd_cache_close_all is called.
> However, only "cacheable" BFDs will be eligible for reopening when
> needed -- and by default BFD decides that if a file descriptor is
> passed in, then it should not be cacheable.  If a non-cacheable BFD's
> file descriptor is closed, there is no offical way to reopen it.
> 
> gdb needs to call bfd_cache_close_all, because some systems cannot
> start an executable when it has an open file descriptor referencing
> it.
> 
> However, gdb also will sometimes passes an open file descriptor to the
> various BFD open functions.  And, due to lazy DWARF reading, gdb may
> also need to reopen these BFDs.
> 
> Rather than having all the callers figure out when exactly to set the
> cacheable flag, I think it makes sense to consolidate this logic into
> the gdb_bfd.c wrapper functions.
> 
> Regression tested on x86-64 Fedora 38.

I think the patch seems sensible (just always marks them as cacheable in
GDB APIs that wrap bfd_open).  It might be worth explicitly noting in the
log perhaps that the relevant functions always provide a filename (even
if they provide an existing file descriptor) which is why it is safe to
enable reopening.

Reviewed-by: John Baldwin <jhb@FreeBSD.org>

-- 
John Baldwin