This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 3/5] remove deleted BFDs from the archive cache
>>>>> "Alan" == Alan Modra <amodra@gmail.com> writes:
Alan> Here's something I hacked together to go a little further than your
Alan> patch and actually close archive member bfds when the archive is
Alan> closed. You'll see I solved the problem of looking up the member BFD
Alan> by saving the filepos in arelt_data. What I haven't done is make this
Alan> all work with thin archives, but that shouldn't be too hard.
Alan> Just a matter of adding a field to struct areltdata instead of
Alan> reusing "origin", and solving the fact that my_archive isn't set for
Alan> thin archives, I think. Care to run with this?
Here's a revised patch.
This patch rolls my various remaining patches into one. I think it
remains pretty clear now.
It adds a couple of new fields to areltdata -- the origin for the
purposes of the cache, and the containing thin archive. I looked a
little at reusing my_archive for thin archives, but this seemed complex.
I tested this under valgrind using a little program that opens an
archive and iterates over the contents, closing each member in turn.
Before the patch, I could provoke either memory leaks or crashes; but
now it is all clean. I ran the test program on itself (to test the
non-archive case), an ordinary archive, a thin archive, and a thin
archive containing a nested archive. I can send the test program if you
want.
Alan, in a private note you mentioned using bfd_read_p in the check in
_bfd_archive_close_and_cleanup. I didn't understand this, so I left it out;
but I can certainly add it if you think it necessary.
Ok?
Tom
2012-08-08 Alan Modra <amodra@gmail.com>
Tom Tromey <tromey@redhat.com>
* archive.c (SECTION Archives): Update documentation.
(_bfd_delete_archive_data): Remove.
(_bfd_get_elt_at_filepos): Set 'thin_archive' and
'cache_origin' fields on new_areldata.
(archive_close_worker, _bfd_archive_close_and_cleanup): New
functions.
* libbfd-in.h (struct areltdata) <cache_origin, thin_archive>:
New fields.
(_bfd_delete_archive_data): Don't declare.
(_bfd_archive_close_and_cleanup): Declare.
(_bfd_generic_close_and_cleanup): Redefine.
* libbfd.h: Rebuild.
* opncls.c (_bfd_delete_bfd): Don't call
_bfd_delete_archive_data.
(bfd_close): Don't close nested thin archives.
diff --git a/bfd/archive.c b/bfd/archive.c
index 1148c11..1b19f00 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -42,11 +42,17 @@ DESCRIPTION
have to read the entire archive if you don't want
to! Read it until you find what you want.
+ A BFD returned by <<bfd_openr_next_archived_file>> can be
+ closed manually with <<bfd_close>>. If you do not close it,
+ then a second iteration through the members of an archive may
+ return the same BFD. If you close the archive BFD, then all
+ the member BFDs will automatically be closed as well.
+
Archive contents of output BFDs are chained through the
- <<next>> pointer in a BFD. The first one is findable through
- the <<archive_head>> slot of the archive. Set it with
- <<bfd_set_archive_head>> (q.v.). A given BFD may be in only one
- open output archive at a time.
+ <<archive_next>> pointer in a BFD. The first one is findable
+ through the <<archive_head>> slot of the archive. Set it with
+ <<bfd_set_archive_head>> (q.v.). A given BFD may be in only
+ one open output archive at a time.
As expected, the BFD archive code is more general than the
archive code of any given environment. BFD archives may
@@ -293,19 +299,6 @@ bfd_set_archive_head (bfd *output_archive, bfd *new_head)
return TRUE;
}
-/* Free the archive hash table, if it exists. */
-
-void
-_bfd_delete_archive_data (bfd *abfd)
-{
- struct artdata *ardata = bfd_ardata (abfd);
-
- BFD_ASSERT (abfd->format == bfd_archive);
-
- if (ardata && ardata->cache)
- htab_delete (ardata->cache);
-}
-
bfd *
_bfd_look_for_bfd_in_cache (bfd *arch_bfd, file_ptr filepos)
{
@@ -678,6 +671,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
n_nfd = bfd_openr (filename, target);
if (n_nfd == NULL)
bfd_set_error (bfd_error_malformed_archive);
+ new_areldata->thin_archive = archive;
}
else
{
@@ -702,6 +696,7 @@ _bfd_get_elt_at_filepos (bfd *archive, file_ptr filepos)
n_nfd->filename = filename;
}
+ new_areldata->cache_origin = filepos;
n_nfd->arelt_data = new_areldata;
/* Copy BFD_COMPRESS and BFD_DECOMPRESS flags. */
@@ -2695,3 +2690,68 @@ coff_write_armap (bfd *arch,
return TRUE;
}
+
+static int
+archive_close_worker (void **slot, void *inf ATTRIBUTE_UNUSED)
+{
+ struct ar_cache *ent = (struct ar_cache *) *slot;
+
+ bfd_close_all_done (ent->arbfd);
+ return 1;
+}
+
+bfd_boolean
+_bfd_archive_close_and_cleanup (bfd *abfd)
+{
+ bfd *archive;
+
+ if (abfd->format == bfd_archive)
+ {
+ bfd *nbfd;
+ bfd *next;
+ htab_t htab;
+
+ /* Close nested archives (if this bfd is a thin archive). */
+ for (nbfd = abfd->nested_archives; nbfd; nbfd = next)
+ {
+ next = nbfd->archive_next;
+ bfd_close (nbfd);
+ }
+
+ htab = bfd_ardata (abfd)->cache;
+ if (htab)
+ {
+ htab_traverse_noresize (htab, archive_close_worker, NULL);
+ htab_delete (htab);
+ bfd_ardata (abfd)->cache = NULL;
+ }
+ }
+
+ if (abfd->my_archive != NULL)
+ archive = abfd->my_archive;
+ else if (arch_eltdata (abfd) != NULL
+ && arch_eltdata (abfd)->thin_archive != NULL)
+ archive = arch_eltdata (abfd)->thin_archive;
+ else
+ archive = NULL;
+ if (archive != NULL)
+ {
+ htab_t htab = bfd_ardata (archive)->cache;
+
+ if (htab)
+ {
+ struct ar_cache ent;
+ void **slot;
+ struct areltdata *ared = arch_eltdata (abfd);
+
+ ent.ptr = ared->cache_origin;
+ slot = htab_find_slot (htab, &ent, NO_INSERT);
+ if (slot != NULL)
+ {
+ BFD_ASSERT (((struct ar_cache *) *slot)->arbfd == abfd);
+ htab_clear_slot (htab, slot);
+ }
+ }
+ }
+ return TRUE;
+}
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 8cdb1c6..a557e36 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -96,6 +96,8 @@ struct areltdata
bfd_size_type extra_size; /* BSD4.4: extra bytes after the header. */
char *filename; /* Null-terminated. */
file_ptr origin; /* For element of a thin archive. */
+ file_ptr cache_origin; /* Origin as recorded in the archive cache. */
+ struct bfd *thin_archive; /* The containing thin archive. */
};
#define arelt_size(bfd) (((struct areltdata *)((bfd)->arelt_data))->parsed_size)
@@ -130,8 +132,6 @@ extern void bfd_release
bfd * _bfd_create_empty_archive_element_shell
(bfd *obfd);
-void _bfd_delete_archive_data
- (bfd *abfd);
bfd * _bfd_look_for_bfd_in_cache
(bfd *, file_ptr);
bfd_boolean _bfd_add_bfd_to_archive_cache
@@ -232,7 +232,9 @@ int bfd_generic_stat_arch_elt
/* Generic routines to use for BFD_JUMP_TABLE_GENERIC. Use
BFD_JUMP_TABLE_GENERIC (_bfd_generic). */
-#define _bfd_generic_close_and_cleanup bfd_true
+#define _bfd_generic_close_and_cleanup _bfd_archive_close_and_cleanup
+extern bfd_boolean _bfd_archive_close_and_cleanup
+ (bfd *);
#define _bfd_generic_bfd_free_cached_info bfd_true
extern bfd_boolean _bfd_generic_new_section_hook
(bfd *, asection *);
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index a1e544f..6b7055a 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -101,6 +101,8 @@ struct areltdata
bfd_size_type extra_size; /* BSD4.4: extra bytes after the header. */
char *filename; /* Null-terminated. */
file_ptr origin; /* For element of a thin archive. */
+ file_ptr cache_origin; /* Origin as recorded in the archive cache. */
+ struct bfd *thin_archive; /* The containing thin archive. */
};
#define arelt_size(bfd) (((struct areltdata *)((bfd)->arelt_data))->parsed_size)
@@ -135,8 +137,6 @@ extern void bfd_release
bfd * _bfd_create_empty_archive_element_shell
(bfd *obfd);
-void _bfd_delete_archive_data
- (bfd *abfd);
bfd * _bfd_look_for_bfd_in_cache
(bfd *, file_ptr);
bfd_boolean _bfd_add_bfd_to_archive_cache
@@ -237,7 +237,9 @@ int bfd_generic_stat_arch_elt
/* Generic routines to use for BFD_JUMP_TABLE_GENERIC. Use
BFD_JUMP_TABLE_GENERIC (_bfd_generic). */
-#define _bfd_generic_close_and_cleanup bfd_true
+#define _bfd_generic_close_and_cleanup _bfd_archive_close_and_cleanup
+extern bfd_boolean _bfd_archive_close_and_cleanup
+ (bfd *);
#define _bfd_generic_bfd_free_cached_info bfd_true
extern bfd_boolean _bfd_generic_new_section_hook
(bfd *, asection *);
diff --git a/bfd/opncls.c b/bfd/opncls.c
index e538981..b2ed9be 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -130,9 +130,6 @@ _bfd_new_bfd_contained_in (bfd *obfd)
static void
_bfd_delete_bfd (bfd *abfd)
{
- if (abfd->format == bfd_archive)
- _bfd_delete_archive_data (abfd);
-
if (abfd->memory)
{
bfd_hash_table_free (&abfd->section_htab);
@@ -711,8 +708,6 @@ bfd_boolean
bfd_close (bfd *abfd)
{
bfd_boolean ret;
- bfd *nbfd;
- bfd *next;
if (bfd_write_p (abfd))
{
@@ -720,13 +715,6 @@ bfd_close (bfd *abfd)
return FALSE;
}
- /* Close nested archives (if this bfd is a thin archive). */
- for (nbfd = abfd->nested_archives; nbfd; nbfd = next)
- {
- next = nbfd->archive_next;
- bfd_close (nbfd);
- }
-
if (! BFD_SEND (abfd, _close_and_cleanup, (abfd)))
return FALSE;