[PATCH 3/3] Further simplify gdb BFD caching

Tom Tromey
2020-01-14T21:10:03Z

This patch wasn't necessary to fix any bug, but while working on the
previous patches I noticed that gdb was calling bfd_stat one extra
time, and that it would be simple to rearrange the code to avoid this
extra call. This also changes gdb so that it no longer caches a BFD
if the stat fails, which seems preferable.

gdb/ChangeLog
2020-01-14 Tom Tromey <tromey@adacore.com>

* gdb_bfd.c (gdb_bfd_data): Remove "mt" parameter, add "st"
parameter. Don't call bfd_stat.
(gdb_bfd_init_data): Replace "mt" parameter with "st".
(gdb_bfd_open): Rearrange and update.
(gdb_bfd_ref): Call bfd_stat.

Change-Id: I48f13f09f59bde49191cb40ee88ed3e18fa7c7d9
---
gdb/ChangeLog | 8 ++++++
gdb/gdb_bfd.c | 69 ++++++++++++++++++++++-----------------------------
2 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 26968396a15..d64630b2362 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -59,26 +59,16 @@ static htab_t all_bfds;

struct gdb_bfd_data
{
- gdb_bfd_data (bfd *abfd, time_t mt)
- : mtime (mt),
- size (bfd_get_size (abfd)),
+ /* Note that if ST is nullptr, then we simply fill in zeroes. */
+ gdb_bfd_data (bfd *abfd, struct stat *st)
+ : mtime (st == nullptr ? 0 : st->st_mtime),
+ size (st == nullptr ? 0 : st->st_size),
+ inode (st == nullptr ? 0 : st->st_ino),
+ device_id (st == nullptr ? 0 : st->st_dev),
relocation_computed (0),
needs_relocations (0),
crc_computed (0)
{
- struct stat buf;
-
- if (bfd_stat (abfd, &buf) == 0)
- {
- inode = buf.st_ino;
- device_id = buf.st_dev;
- }
- else
- {
- /* The stat failed. */
- inode = 0;
- device_id = 0;
- }
}

~gdb_bfd_data ()
@@ -380,7 +370,7 @@ gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream,
BFD. */

static void
-gdb_bfd_init_data (struct bfd *abfd, time_t mtime)
+gdb_bfd_init_data (struct bfd *abfd, struct stat *st)
{
struct gdb_bfd_data *gdata;
void **slot;
@@ -390,7 +380,7 @@ gdb_bfd_init_data (struct bfd *abfd, time_t mtime)
/* Ask BFD to decompress sections in bfd_get_full_section_contents. */
abfd->flags |= BFD_DECOMPRESS;

- gdata = new gdb_bfd_data (abfd, mtime);
+ gdata = new gdb_bfd_data (abfd, st);
bfd_set_usrdata (abfd, gdata);
bfd_alloc_data (abfd);

@@ -405,10 +395,7 @@ gdb_bfd_init_data (struct bfd *abfd, time_t mtime)
gdb_bfd_ref_ptr
gdb_bfd_open (const char *name, const char *target, int fd)
{
- hashval_t hash;
- void **slot;
bfd *abfd;
- struct gdb_bfd_cache_search search;
struct stat st;

if (is_target_filename (name))
@@ -428,10 +415,6 @@ gdb_bfd_open (const char *name, const char *target, int fd)
name += strlen (TARGET_SYSROOT_PREFIX);
}

- if (gdb_bfd_cache == NULL)
- gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
- xcalloc, xfree);
-
if (fd == -1)
{
fd = gdb_open_cloexec (name, O_RDONLY | O_BINARY, 0);
@@ -449,29 +432,31 @@ gdb_bfd_open (const char *name, const char *target, int fd)
if (abfd == NULL)
return NULL;

- search.filename = name;
+ struct stat *st_ptr = &st;
if (bfd_stat (abfd, &st) < 0)
{
- /* Weird situation here. */
- search.mtime = 0;
- search.size = 0;
- search.inode = 0;
- search.device_id = 0;
+ /* Weird situation here -- don't cache if we can't stat. */
+ st_ptr = nullptr;
}
- else
+
+ if (bfd_sharing && st_ptr != nullptr)
{
+ if (gdb_bfd_cache == NULL)
+ gdb_bfd_cache = htab_create_alloc (1, hash_bfd, eq_bfd, NULL,
+ xcalloc, xfree);
+
+ struct gdb_bfd_cache_search search;
+ search.filename = name;
search.mtime = st.st_mtime;
search.size = st.st_size;
search.inode = st.st_ino;
search.device_id = st.st_dev;
- }

- /* Note that this must compute the same result as hash_bfd. */
- hash = htab_hash_string (name);
+ /* Note that this must compute the same result as hash_bfd. */
+ hashval_t hash = htab_hash_string (name);

- if (bfd_sharing)
- {
- slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
+ void **slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash,
+ INSERT);
if (*slot != nullptr)
{
bfd_close (abfd);
@@ -500,7 +485,7 @@ gdb_bfd_open (const char *name, const char *target, int fd)
and since we already entered it into the hash table using this
mtime, if the file changed at the wrong moment, the race would
lead to a hash table corruption. */
- gdb_bfd_init_data (abfd, search.mtime);
+ gdb_bfd_init_data (abfd, st_ptr);
return gdb_bfd_ref_ptr (abfd);
}

@@ -572,7 +557,11 @@ gdb_bfd_ref (struct bfd *abfd)
return;
}

- gdb_bfd_init_data (abfd, bfd_get_mtime (abfd));
+ struct stat st, *st_ptr = &st;
+ if (bfd_stat (abfd, &st) < 0)
+ st_ptr = nullptr;
+
+ gdb_bfd_init_data (abfd, st_ptr);
}

/* See gdb_bfd.h. */
--
2.21.1