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

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

[Andrew Burgess]

This is a revision of a patch set that I posted here:
  https://sourceware.org/ml/gdb-patches/2015-04/msg00471.html

The differences in each patch are as follows:

  1/3

  I hope that I addressed everyones comments.  I added comparison of
  inode and device-id fields as suggested by Pedro.

  Gary pointed out a few places where we fake the stat results,
  however, these are all for specialised targets that I don't think
  will be using the bfd caching anyway, the caching is only used for
  local files, ones that can be targeted with a local 'stat' command.
  
  It was also pointed out that Windows does not set the inode field of
  the stat struture.  Pedro semi-suggested that for such targets
  caching should be disabled, while Eli pointed out Windows has an
  alternative API to get the similar data.

  In the end I did neither of these things for the following reasons:

  - Caching provides a general benefit in the common use case.  The
    problem area is basically an edge case in automatted testing.
    Punishing all users (on Windows) because of such a small edge
    case, and one that has existed for a long time feels harsh.

  - To slightly mitigate the above I have left the check of file size
    in place.  In my experience this is often enough to make the
    problem invisible.

  - I don't have a Windows machine for development or testing, but if
    someone else wanted to make use of the Windows API to supply an
    inode like result, this can be easily filled in within
    gdb_bfd_open.

  2/3
  3/3

  Added documentation to the gdb.texinfo file.  I created a whole new
  section to talk about file caching.  I don't know if I filled in
  enough detail, but hopefully what I've written makes sense and is a
  starting point.  I placed all the command documentation there, even
  the maintenance commands, there is precedent for having maintenance
  commands mixed in with other commands so I hope that that will also
  be acceptable.

Andrew Burgess (3):
  gdb: Improve cache matching criteria for the bfd cache.
  gdb: New maintenance command to disable bfd sharing.
  gdb: Add debug tracing for bfd cache activity.

 gdb/ChangeLog       |  26 ++++++++++++
 gdb/NEWS            |   8 ++++
 gdb/doc/ChangeLog   |  11 +++++
 gdb/doc/gdb.texinfo |  45 +++++++++++++++++---
 gdb/gdb_bfd.c       | 118 +++++++++++++++++++++++++++++++++++++++++++++++++---
 5 files changed, 197 insertions(+), 11 deletions(-)

-- 
2.4.0

[PATCH v2 2/3] gdb: New maintenance command to disable bfd sharing.

[Andrew Burgess]

In some rare maintainer cases it is desirable to be able to disable bfd
sharing.  This patch adds new commands maintenance set/show commands for
bfd-sharing, allowing gdb's bfd cache to be turned off.

gdb/ChangeLog:

	* gdb_bfd.c (bfd_sharing): New variable.
	(gdb_bfd_open): Check bfd_sharing variable.
	(_initialize_gdb_bfd): Add new set/show command.
	* NEWS: Mention new command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Maintenance Commands): Move documentation of "main
	info bfds" to...
	(File Caching): A New section.  Outline bfd caching, and add new
	description for "main set/show bfd-sharing".
---
 gdb/ChangeLog       |  7 +++++++
 gdb/NEWS            |  4 ++++
 gdb/doc/ChangeLog   |  7 +++++++
 gdb/doc/gdb.texinfo | 35 ++++++++++++++++++++++++++++++-----
 gdb/gdb_bfd.c       | 29 +++++++++++++++++++++++++----
 5 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d920ea0..0f73f08 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb_bfd.c (bfd_sharing): New variable.
+	(gdb_bfd_open): Check bfd_sharing variable.
+	(_initialize_gdb_bfd): Add new set/show command.
+	* NEWS: Mention new command.
+
+2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb_bfd.c (struct gdb_bfd_data): Add size, inode, and device id
 	field.
 	(struct gdb_bfd_cache_search): Likewise.
diff --git a/gdb/NEWS b/gdb/NEWS
index 3fe6036..87fc14e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -16,6 +16,10 @@ maint show target-non-stop
   "set non-stop" is "off".  The default is "auto", meaning non-stop
   mode is enabled if supported by the target.
 
+maint set bfd-sharing
+maint show bfd-sharing
+  Control the reuse of bfd objects.
+
 *** Changes in GDB 7.10
 
 * Support for process record-replay and reverse debugging on aarch64*-linux*
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 846ac24..9d553db 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,10 @@
+2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Maintenance Commands): Move documentation of "main
+	info bfds" to...
+	(File Caching): A New section.  Outline bfd caching, and add new
+	description for "main set/show bfd-sharing".
+
 2015-08-07  Pedro Alves  <palves@redhat.com>
 
 	* gdb.texinfo (Maintenance Commands): Document "maint set/show
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 900970b..3cfb1a7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18190,6 +18190,36 @@ Set whether a source file may have multiple base names.
 Show whether a source file may have multiple base names.
 @end table
 
+@node File Caching
+@section File Caching
+@cindex caching of opened files
+@cindex caching of bfd objects
+
+To speed up file loading, and reduce memory usage, @value{GDBN} will
+reuse the @code{bfd} objects used to track open files.  @xref{Top, ,
+BFD, bfd, The Binary File Descriptor Library}.  The following commands
+allow visibility and control of the caching beaviour.
+
+@table @code
+@kindex maint info bfds
+@item maint info bfds
+This prints information about each @code{bfd} object that is known to
+@value{GDBN}.
+
+@kindex maint set bfd-sharing
+@kindex maint show bfd-sharing
+@kindex bfd caching
+@item maint set bfd-sharing
+@item maint show bfd-sharing
+Control whether @code{bfd} objects can be shared.  When sharing is
+enabled @value{GDBN} will reuse already open @code{bfd} objects rather
+than reopening the same file.  Turning sharing off does not cause
+already shared @code{bfd} objects to be unshared, but all future files
+that are opened will create a new @code{bfd} object.  Similarly,
+re-enabling sharing will not cause multiple existing @code{bfd}
+objects to be collapsed into a single shared @code{bfd} object.
+@end table
+
 @node Separate Debug Files
 @section Debugging Information in Separate Files
 @cindex separate debugging information files
@@ -33987,11 +34017,6 @@ Shared library events.
 
 @end table
 
-@kindex maint info bfds
-@item maint info bfds
-This prints information about each @code{bfd} object that is known to
-@value{GDBN}.  @xref{Top, , BFD, bfd, The Binary File Descriptor Library}.
-
 @kindex maint info btrace
 @item maint info btrace
 Pint information about raw branch tracing data.
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index ee29531..14fdf43 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -111,6 +111,11 @@ DEFINE_REGISTRY (bfd, GDB_BFD_DATA_ACCESSOR)
 
 static htab_t gdb_bfd_cache;
 
+/* When true gdb will reuse an existing bfd object if the filename,
+   modification time, and file size all match.  */
+
+static int bfd_sharing;
+
 /* The type of an object being looked up in gdb_bfd_cache.  We use
    htab's capability of storing one kind of object (BFD in this case)
    and using a different sort of object for searching.  */
@@ -394,7 +399,7 @@ gdb_bfd_open (const char *name, const char *target, int fd)
      opening the BFD may fail; and this would violate hashtab
      invariants.  */
   abfd = htab_find_with_hash (gdb_bfd_cache, &search, hash);
-  if (abfd != NULL)
+  if (bfd_sharing && abfd != NULL)
     {
       close (fd);
       gdb_bfd_ref (abfd);
@@ -405,9 +410,12 @@ gdb_bfd_open (const char *name, const char *target, int fd)
   if (abfd == NULL)
     return NULL;
 
-  slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
-  gdb_assert (!*slot);
-  *slot = abfd;
+  if (bfd_sharing)
+    {
+      slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
+      gdb_assert (!*slot);
+      *slot = abfd;
+    }
 
   gdb_bfd_ref (abfd);
   return abfd;
@@ -942,4 +950,17 @@ _initialize_gdb_bfd (void)
   add_cmd ("bfds", class_maintenance, maintenance_info_bfds, _("\
 List the BFDs that are currently open."),
 	   &maintenanceinfolist);
+
+  add_setshow_boolean_cmd ("bfd-sharing", no_class,
+			   &bfd_sharing, _("\
+Set whether gdb will share bfds that appear to be the same file."), _("\
+Show whether gdb will share bfds that appear to be the same file."), _("\
+When enabled gdb will reuse existing bfds rather than reopening the\n\
+same file.  To decide if two files are the same then gdb compares the\n\
+filename, file size, file modification time, and file inode."),
+			   NULL,
+			   NULL,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
+  bfd_sharing = 1;
 }
-- 
2.4.0

Re: [PATCH v2 2/3] gdb: New maintenance command to disable bfd sharing.

[Eli Zaretskii]

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Thu, 13 Aug 2015 13:44:59 +0100
> 
> In some rare maintainer cases it is desirable to be able to disable bfd
> sharing.  This patch adds new commands maintenance set/show commands for
> bfd-sharing, allowing gdb's bfd cache to be turned off.
> 
> gdb/ChangeLog:
> 
> 	* gdb_bfd.c (bfd_sharing): New variable.
> 	(gdb_bfd_open): Check bfd_sharing variable.
> 	(_initialize_gdb_bfd): Add new set/show command.
> 	* NEWS: Mention new command.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Maintenance Commands): Move documentation of "main
> 	info bfds" to...
> 	(File Caching): A New section.  Outline bfd caching, and add new
> 	description for "main set/show bfd-sharing".

OK for the documentation parts, with one comment:

> +Control whether @code{bfd} objects can be shared.  When sharing is
> +enabled @value{GDBN} will reuse already open @code{bfd} objects rather
          ^
Please insert a comma where indicated.

Thanks.

Re: [PATCH v2 2/3] gdb: New maintenance command to disable bfd sharing.

[Pedro Alves]

On 08/13/2015 01:44 PM, Andrew Burgess wrote:
> In some rare maintainer cases it is desirable to be able to disable bfd
> sharing.  This patch adds new commands maintenance set/show commands for
> bfd-sharing, allowing gdb's bfd cache to be turned off.


>  * Support for process record-replay and reverse debugging on aarch64*-linux*
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 846ac24..9d553db 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,10 @@
> +2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.texinfo (Maintenance Commands): Move documentation of "main
> +	info bfds" to...
> +	(File Caching): A New section.  Outline bfd caching, and add new
> +	description for "main set/show bfd-sharing".
> +
>  2015-08-07  Pedro Alves  <palves@redhat.com>
>  
>  	* gdb.texinfo (Maintenance Commands): Document "maint set/show
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 900970b..3cfb1a7 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -18190,6 +18190,36 @@ Set whether a source file may have multiple base names.
>  Show whether a source file may have multiple base names.
>  @end table
>  
> +@node File Caching
> +@section File Caching
> +@cindex caching of opened files
> +@cindex caching of bfd objects
> +
> +To speed up file loading, and reduce memory usage, @value{GDBN} will
> +reuse the @code{bfd} objects used to track open files.  @xref{Top, ,
> +BFD, bfd, The Binary File Descriptor Library}.  The following commands
> +allow visibility and control of the caching beaviour.

typo "beaviour".  (While at it, I think the manual is meant to
standardize on USA spelling, thus "behavior".)

> +
> +@table @code
> +@kindex maint info bfds
> +@item maint info bfds
> +This prints information about each @code{bfd} object that is known to
> +@value{GDBN}.
> +
> +@kindex maint set bfd-sharing
> +@kindex maint show bfd-sharing
> +@kindex bfd caching
> +@item maint set bfd-sharing
> +@item maint show bfd-sharing
> +Control whether @code{bfd} objects can be shared.  When sharing is
> +enabled @value{GDBN} will reuse already open @code{bfd} objects rather

I believe present tense is preferred in the manual, like:

s/will reuse/reuses/

> +than reopening the same file.  Turning sharing off does not cause
> +already shared @code{bfd} objects to be unshared, but all future files
> +that are opened will create a new @code{bfd} object.  Similarly,
> +re-enabling sharing will not cause multiple existing @code{bfd}

...re-enabling sharing does not cause...


> +objects to be collapsed into a single shared @code{bfd} object.
> +@end table
> +
>  @node Separate Debug Files
>  @section Debugging Information in Separate Files
>  @cindex separate debugging information files
> @@ -33987,11 +34017,6 @@ Shared library events.


> @@ -111,6 +111,11 @@ DEFINE_REGISTRY (bfd, GDB_BFD_DATA_ACCESSOR)
>  
>  static htab_t gdb_bfd_cache;
>  
> +/* When true gdb will reuse an existing bfd object if the filename,
> +   modification time, and file size all match.  */
> +
> +static int bfd_sharing;

Initialize it here?  Like:

 static int bfd_sharing = 1;

>    gdb_bfd_ref (abfd);
>    return abfd;
> @@ -942,4 +950,17 @@ _initialize_gdb_bfd (void)
>    add_cmd ("bfds", class_maintenance, maintenance_info_bfds, _("\
>  List the BFDs that are currently open."),
>  	   &maintenanceinfolist);
> +
> +  add_setshow_boolean_cmd ("bfd-sharing", no_class,
> +			   &bfd_sharing, _("\
> +Set whether gdb will share bfds that appear to be the same file."), _("\
> +Show whether gdb will share bfds that appear to be the same file."), _("\
> +When enabled gdb will reuse existing bfds rather than reopening the\n\
> +same file.  To decide if two files are the same then gdb compares the\n\
> +filename, file size, file modification time, and file inode."),
> +			   NULL,
> +			   NULL,

Please add a show hook, for i18n.

> +			   &maintenance_set_cmdlist,
> +			   &maintenance_show_cmdlist);
> +  bfd_sharing = 1;

See above.

>  }
> 

Otherwise LGTM.

Thanks,
Pedro Alves

[PATCH v2 3/3] gdb: Add debug tracing for bfd cache activity.

[Andrew Burgess]

This patch adds a new debug flag bfd-cache, which when set to non-zero
produces debugging log messages relating to gdb's bfd cache.

gdb/ChangeLog:

	* gdb_bfd.c (debug_bfd_cache): New variable.
	(gdb_bfd_open): Add debug logging.
	(gdb_bfd_ref): Likewise.
	(gdb_bfd_unref): Likewise.
	(_initialize_gdb_bfd): Add new set/show command.
	* NEWS: Mention new command.

gdb/doc/ChngeLog:

	* gdb.texinfo (File Caching): Document "set/show debug bfd-cache".
---
 gdb/ChangeLog       |  9 +++++++++
 gdb/NEWS            |  4 ++++
 gdb/doc/ChangeLog   |  4 ++++
 gdb/doc/gdb.texinfo | 10 ++++++++++
 gdb/gdb_bfd.c       | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0f73f08..3ef6d0e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb_bfd.c (debug_bfd_cache): New variable.
+	(gdb_bfd_open): Add debug logging.
+	(gdb_bfd_ref): Likewise.
+	(gdb_bfd_unref): Likewise.
+	(_initialize_gdb_bfd): Add new set/show command.
+	* NEWS: Mention new command.
+
+2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb_bfd.c (bfd_sharing): New variable.
 	(gdb_bfd_open): Check bfd_sharing variable.
 	(_initialize_gdb_bfd): Add new set/show command.
diff --git a/gdb/NEWS b/gdb/NEWS
index 87fc14e..2451ac3 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -20,6 +20,10 @@ maint set bfd-sharing
 maint show bfd-sharing
   Control the reuse of bfd objects.
 
+set debug bfd-cache
+show debug bfd-cache
+  Control display of debugging info regarding bfd caching.
+
 *** Changes in GDB 7.10
 
 * Support for process record-replay and reverse debugging on aarch64*-linux*
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 9d553db..2fe1dd9 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,9 @@
 2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.texinfo (File Caching): Document "set/show debug bfd-cache".
+
+2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb.texinfo (Maintenance Commands): Move documentation of "main
 	info bfds" to...
 	(File Caching): A New section.  Outline bfd caching, and add new
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3cfb1a7..0a7ced4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18218,6 +18218,16 @@ already shared @code{bfd} objects to be unshared, but all future files
 that are opened will create a new @code{bfd} object.  Similarly,
 re-enabling sharing will not cause multiple existing @code{bfd}
 objects to be collapsed into a single shared @code{bfd} object.
+
+@kindex set debug bfd-cache @var{level}
+@kindex bfd caching
+@item set debug bfd-cache @var{level}
+Turns on debugging of the bfd cache, setting the level to @var{level}.
+
+@kindex show debug bfd-cache
+@kindex bfd caching
+@item show debug bfd-cache
+Show the current debugging level of the bfd cache.
 @end table
 
 @node Separate Debug Files
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 14fdf43..e252999 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -116,6 +116,10 @@ static htab_t gdb_bfd_cache;
 
 static int bfd_sharing;
 
+/* When non-zero debugging of the bfd caches is enabled.  */
+
+static unsigned int debug_bfd_cache;
+
 /* The type of an object being looked up in gdb_bfd_cache.  We use
    htab's capability of storing one kind of object (BFD in this case)
    and using a different sort of object for searching.  */
@@ -401,6 +405,11 @@ gdb_bfd_open (const char *name, const char *target, int fd)
   abfd = htab_find_with_hash (gdb_bfd_cache, &search, hash);
   if (bfd_sharing && abfd != NULL)
     {
+      if (debug_bfd_cache)
+	fprintf_unfiltered (gdb_stdlog,
+			    "Reusing cached bfd %s for %s\n",
+			    host_address_to_string (abfd),
+			    bfd_get_filename (abfd));
       close (fd);
       gdb_bfd_ref (abfd);
       return abfd;
@@ -410,6 +419,12 @@ gdb_bfd_open (const char *name, const char *target, int fd)
   if (abfd == NULL)
     return NULL;
 
+  if (debug_bfd_cache)
+    fprintf_unfiltered (gdb_stdlog,
+			"Creating new bfd %s for %s\n",
+			host_address_to_string (abfd),
+			bfd_get_filename (abfd));
+
   if (bfd_sharing)
     {
       slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash, INSERT);
@@ -478,6 +493,12 @@ gdb_bfd_ref (struct bfd *abfd)
 
   gdata = bfd_usrdata (abfd);
 
+  if (debug_bfd_cache)
+    fprintf_unfiltered (gdb_stdlog,
+			"Increase reference count on bfd %s (%s)\n",
+			host_address_to_string (abfd),
+			bfd_get_filename (abfd));
+
   if (gdata != NULL)
     {
       gdata->refc += 1;
@@ -531,7 +552,20 @@ gdb_bfd_unref (struct bfd *abfd)
 
   gdata->refc -= 1;
   if (gdata->refc > 0)
-    return;
+    {
+      if (debug_bfd_cache)
+	fprintf_unfiltered (gdb_stdlog,
+			    "Decrease reference count on bfd %s (%s)\n",
+			    host_address_to_string (abfd),
+			    bfd_get_filename (abfd));
+      return;
+    }
+
+  if (debug_bfd_cache)
+    fprintf_unfiltered (gdb_stdlog,
+			"Delete final reference count on bfd %s (%s)\n",
+			host_address_to_string (abfd),
+			bfd_get_filename (abfd));
 
   archive_bfd = gdata->archive_bfd;
   search.filename = bfd_get_filename (abfd);
@@ -963,4 +997,13 @@ filename, file size, file modification time, and file inode."),
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
   bfd_sharing = 1;
+
+  add_setshow_zuinteger_cmd ("bfd-cache", class_maintenance,
+			     &debug_bfd_cache, _("\
+Set bfd cache debugging."), _("\
+Show bfd cache debugging."), _("\
+When non-zero, bfd cache specific debugging is enabled."),
+			     NULL,
+			     NULL,
+			     &setdebuglist, &showdebuglist);
 }
-- 
2.4.0

Re: [PATCH v2 3/3] gdb: Add debug tracing for bfd cache activity.

[Pedro Alves]

On 08/13/2015 01:45 PM, Andrew Burgess wrote:

> gdb/doc/ChngeLog:

( typo on commit log )

> +  add_setshow_zuinteger_cmd ("bfd-cache", class_maintenance,
> +			     &debug_bfd_cache, _("\
> +Set bfd cache debugging."), _("\
> +Show bfd cache debugging."), _("\
> +When non-zero, bfd cache specific debugging is enabled."),
> +			     NULL,
> +			     NULL,

Add show hook, for i18n.

Otherwise LGTM.

Thanks,
Pedro Alves

[PATCH v2 1/3] gdb: Improve cache matching criteria for the bfd cache.

[Andrew Burgess]

Within gdb open bfd objects are reused where possible if an attempt is
made to reopen a file that is already being debugged.  To spot if the on
disc file has changed gdb currently examines the mtime of the file and
compares it to the mtime of the open bfd in the cache.

A problem exists when the on disc file is being rapidly regenerated, as
happens, for example, with automated testing.  In some cases the file is
generated so quickly that the mtime appears not to change, while the on
disc file has changed.

This patch extends the bfd cache to also hold the file size of the file,
the inode of the file, and the device id of the file; gdb can then
compare filename, file size, mtime, inode, and device id to determine if
an existing bfd object can be reused.

gdb/ChangeLog:

	* gdb_bfd.c (struct gdb_bfd_data): Add size, inode, and device id
	field.
	(struct gdb_bfd_cache_search): Likewise.
	(eq_bfd): Compare the size, inode, and device id fields.
	(gdb_bfd_open): Initialise the size, inode, and device id fields.
	(gdb_bfd_ref): Likewise.
	(gdb_bfd_unref): Likewise.
---
 gdb/ChangeLog | 10 ++++++++++
 gdb/gdb_bfd.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a104670..d920ea0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2015-08-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb_bfd.c (struct gdb_bfd_data): Add size, inode, and device id
+	field.
+	(struct gdb_bfd_cache_search): Likewise.
+	(eq_bfd): Compare the size, inode, and device id fields.
+	(gdb_bfd_open): Initialise the size, inode, and device id fields.
+	(gdb_bfd_ref): Likewise.
+	(gdb_bfd_unref): Likewise.
+
 2015-08-10  Doug Evans  <dje@google.com>
 	    Keith Seitz  <keiths@redhat.com>
 
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 1781d80..ee29531 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -69,6 +69,15 @@ struct gdb_bfd_data
   /* The mtime of the BFD at the point the cache entry was made.  */
   time_t mtime;
 
+  /* The file size (in bytes) at the point the cache entry was made.  */
+  off_t size;
+
+  /* The inode of the file at the point the cache entry was made.  */
+  ino_t inode;
+
+  /* The device id of the file at the point the cache entry was made.  */
+  dev_t device_id;
+
   /* This is true if we have determined whether this BFD has any
      sections requiring relocation.  */
   unsigned int relocation_computed : 1;
@@ -112,6 +121,12 @@ struct gdb_bfd_cache_search
   const char *filename;
   /* The mtime.  */
   time_t mtime;
+  /* The file size (in bytes).  */
+  off_t size;
+  /* The inode of the file.  */
+  ino_t inode;
+  /* The device id of the file.  */
+  dev_t device_id;
 };
 
 /* A hash function for BFDs.  */
@@ -136,6 +151,9 @@ eq_bfd (const void *a, const void *b)
   struct gdb_bfd_data *gdata = bfd_usrdata (abfd);
 
   return (gdata->mtime == s->mtime
+	  && gdata->size == s->size
+	  && gdata->inode == s->inode
+	  && gdata->device_id == s->device_id
 	  && strcmp (bfd_get_filename (abfd), s->filename) == 0);
 }
 
@@ -358,9 +376,17 @@ gdb_bfd_open (const char *name, const char *target, int fd)
     {
       /* Weird situation here.  */
       search.mtime = 0;
+      search.size = 0;
+      search.inode = 0;
+      search.device_id = 0;
     }
   else
-    search.mtime = st.st_mtime;
+    {
+      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);
@@ -435,6 +461,7 @@ gdb_bfd_close_or_warn (struct bfd *abfd)
 void
 gdb_bfd_ref (struct bfd *abfd)
 {
+  struct stat buf;
   struct gdb_bfd_data *gdata;
   void **slot;
 
@@ -455,7 +482,19 @@ 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;
+  if (bfd_stat (abfd, &buf) == 0)
+    {
+      gdata->inode = buf.st_ino;
+      gdata->device_id = buf.st_dev;
+    }
+  else
+    {
+      /* The stat failed.  */
+      gdata->inode = 0;
+      gdata->device_id = 0;
+    }
   bfd_usrdata (abfd) = gdata;
 
   bfd_alloc_data (abfd);
@@ -495,6 +534,9 @@ gdb_bfd_unref (struct bfd *abfd)
       void **slot;
 
       search.mtime = gdata->mtime;
+      search.size = gdata->size;
+      search.inode = gdata->inode;
+      search.device_id = gdata->device_id;
       slot = htab_find_slot_with_hash (gdb_bfd_cache, &search, hash,
 				       NO_INSERT);
 
-- 
2.4.0

Re: [PATCH v2 1/3] gdb: Improve cache matching criteria for the bfd cache.

[Pedro Alves]

On 08/13/2015 01:44 PM, Andrew Burgess wrote:
> Within gdb open bfd objects are reused where possible if an attempt is
> made to reopen a file that is already being debugged.  To spot if the on
> disc file has changed gdb currently examines the mtime of the file and
> compares it to the mtime of the open bfd in the cache.
> 
> A problem exists when the on disc file is being rapidly regenerated, as
> happens, for example, with automated testing.  In some cases the file is
> generated so quickly that the mtime appears not to change, while the on
> disc file has changed.
> 
> This patch extends the bfd cache to also hold the file size of the file,
> the inode of the file, and the device id of the file; gdb can then
> compare filename, file size, mtime, inode, and device id to determine if
> an existing bfd object can be reused.
> 
> gdb/ChangeLog:
> 
> 	* gdb_bfd.c (struct gdb_bfd_data): Add size, inode, and device id
> 	field.
> 	(struct gdb_bfd_cache_search): Likewise.
> 	(eq_bfd): Compare the size, inode, and device id fields.
> 	(gdb_bfd_open): Initialise the size, inode, and device id fields.
> 	(gdb_bfd_ref): Likewise.
> 	(gdb_bfd_unref): Likewise.

OK.

Thanks,
Pedro Alves