Re: [PATCH 1/2] gdb: move all bfd_cache_close_all calls in gdb_bfd.c

[Tom Tromey <tom@tromey.com>]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Better I think would be to track down where the bfd is being opened
Andrew> and add a corresponding bfd_cache_close_all() call elsewhere in GDB
Andrew> once we've finished doing whatever it is that caused us to open the
Andrew> bfd in the first place.

I had contemplated this solution as well, though using bfd_cache_close
on the specific BFD, instead.

Like you point out, though, it seems finicky to get right.  We could
move up the call chain and add the close calls at some higher level, but
even that seems hard to be assured of.

Andrew>   (a) Just before we display a GDB prompt.  We display a prompt after
Andrew>   completing a command, and GDB is about to enter an idle state
Andrew>   waiting for further input from the user (or in async mode, for an
Andrew>   inferior event).  If while we are in this idle state the user
Andrew>   changes the on-disk file(s) then we would like GDB to notice this
Andrew>   the next time it leaves its idle state, e.g. the next time the user
Andrew>   executes a command, or when an inferior event arrives,

Andrew>   (b) When we resume the inferior.  In synchronous mode, resuming the
Andrew>   inferior is another time when GDB is blocked and sitting idle, but
Andrew>   in this case we don't display a prompt.  As with (a) above, when an
Andrew>   inferior event arrives we want GDB to notice any changes to on-disk
Andrew>   files.

I've been working on moving DWARF reading to the background.  For the
most part, your plan won't be an issue -- gdb pre-reads the relevant
section data on the main thread and doesn't require the BFD to be open
when scanning.

However, there is one case where this will change: opening dwo files.
Now, the way the background reading is implemented, the worst case here
is some inefficiency: the main thread might close all the BFDs, and then
the background reader might immediately reopen one.

Maybe this isn't something to worry about?  I could make the background
reader also call bfd_cache_close for the DWO BFDs, to ensure they are
closed.  (This only really matters for the "rebuild" case, not any sort
of exec case, because DWOs aren't really part of the inferior.)

Andrew> Right now, the only other users of the observers that I'm connecting
Andrew> too are the extension languages, specifically, Python.  I suspect it
Andrew> must be possible for Python to carry out actions that can cause the
Andrew> bfd cache to be populated, so maybe there is a risk here.

Andrew> To counter this risk, we could move the bfd_cache_close_all() calls
Andrew> out of observers, and move them into GDB core close too, but after the
Andrew> two observers in questions are actually called, so into
Andrew> top_level_prompt for the before_prompt observer and into
Andrew> notify_target_resumed for the target_resumed observer.

Andrew> Another choice would be to add a bfd_cache_close_all() into
Andrew> gdbpy_enter::~gdbpy_enter, so whenever we call into a Python
Andrew> extension, we always clear the bfd cache once we're done.

Andrew> For now I haven't made either of these changes, maybe I'm worrying
Andrew> about nothing?  I'd appreciate peoples thoughts.

Do we know of specific Python APIs that could cause problems?

I guess anything that reads memory if trust-readonly-sections is
enabled?  That's the only one I could think of offhand.

Maybe we need a combo approach where some calls call bfd_cache_close at
the end.

Tom