ld plugin bfd_make_readable leak
Checks
Commit Message
bfd_make_readable leaks memory that could be freed by
_free_cached_info except that does too much in releasing all bfd
memory. (The fact that we had to hack around keeping the bfd filename
also indicates that releasing all bfd memory was too much.) So this
patch moves code releasing bfd_alloc'd memory to the COFF
_free_cached_info, where the syms and suchlike are released. This is
the memory that archive handling wants to release in the call there to
bfd_free_cached_info.
* coffgen.c (_bfd_coff_free_cached_info): Release syms.
* opncls.c (_bfd_new_bfd): Correct error return path.
(_bfd_free_cached_info): Don't kill all abfd->memory.
(_bfd_delete_bfd): Adjust fallback for bfd_free_cached_info.
(bfd_make_readable): Call target bfd_free_cached_info and
_bfd_free_cached_info plus reinstate section_htab.
Comments
Commit 3097045a18a8 runs into an abort in objalloc_free_block when the
memory being bfd_release'd wasn't bfd_alloc'd. Fix that.
* coffgen.c (_bfd_coff_free_cached_info): Don't release
raw_syments when obj_coff_keep_raw_syms.
* libcoff-in.h (obj_coff_keep_raw_syms): Define.
(struct coff_tdata): Add keep_raw_syms.
* peicode.h (pe_ILF_build_a_bfd): Set obj_coff_keep_raw_syms.
* libcoff.h: Regenerate.
diff --git a/bfd/coffgen.c b/bfd/coffgen.c
index c4026e96afb..c734f058892 100644
--- a/bfd/coffgen.c
+++ b/bfd/coffgen.c
@@ -3299,11 +3299,13 @@ _bfd_coff_free_cached_info (bfd *abfd)
/* Free raw syms, and any other data bfd_alloc'd after raw syms
are read. */
- if (obj_raw_syments (abfd))
- bfd_release (abfd, obj_raw_syments (abfd));
- obj_raw_syments (abfd) = NULL;
- obj_symbols (abfd) = NULL;
- obj_convert (abfd) = NULL;
+ if (!obj_coff_keep_raw_syms (abfd) && obj_raw_syments (abfd))
+ {
+ bfd_release (abfd, obj_raw_syments (abfd));
+ obj_raw_syments (abfd) = NULL;
+ obj_symbols (abfd) = NULL;
+ obj_convert (abfd) = NULL;
+ }
}
return _bfd_generic_bfd_free_cached_info (abfd);
diff --git a/bfd/libcoff-in.h b/bfd/libcoff-in.h
index 51f964c4fd2..1410b08f6ae 100644
--- a/bfd/libcoff-in.h
+++ b/bfd/libcoff-in.h
@@ -40,6 +40,7 @@ extern "C" {
#define obj_relocbase(bfd) (coff_data (bfd)->relocbase)
#define obj_raw_syments(bfd) (coff_data (bfd)->raw_syments)
#define obj_raw_syment_count(bfd) (coff_data (bfd)->raw_syment_count)
+#define obj_coff_keep_raw_syms(bfd) (coff_data (bfd)->keep_raw_syms)
#define obj_convert(bfd) (coff_data (bfd)->conversion_table)
#define obj_conv_table_size(bfd) (coff_data (bfd)->conv_table_size)
#define obj_coff_external_syms(bfd) (coff_data (bfd)->external_syms)
@@ -91,6 +92,8 @@ typedef struct coff_tdata
backend flag _bfd_coff_long_section_names. */
bool long_section_names;
+ /* If this is TRUE, raw_syments may not be released. */
+ bool keep_raw_syms;
/* If this is TRUE, the external_syms may not be freed. */
bool keep_syms;
/* If this is TRUE, the strings may not be freed. */
diff --git a/bfd/libcoff.h b/bfd/libcoff.h
index 947dbeb67a0..d0cfd09709a 100644
--- a/bfd/libcoff.h
+++ b/bfd/libcoff.h
@@ -44,6 +44,7 @@ extern "C" {
#define obj_relocbase(bfd) (coff_data (bfd)->relocbase)
#define obj_raw_syments(bfd) (coff_data (bfd)->raw_syments)
#define obj_raw_syment_count(bfd) (coff_data (bfd)->raw_syment_count)
+#define obj_coff_keep_raw_syms(bfd) (coff_data (bfd)->keep_raw_syms)
#define obj_convert(bfd) (coff_data (bfd)->conversion_table)
#define obj_conv_table_size(bfd) (coff_data (bfd)->conv_table_size)
#define obj_coff_external_syms(bfd) (coff_data (bfd)->external_syms)
@@ -95,6 +96,8 @@ typedef struct coff_tdata
backend flag _bfd_coff_long_section_names. */
bool long_section_names;
+ /* If this is TRUE, raw_syments may not be released. */
+ bool keep_raw_syms;
/* If this is TRUE, the external_syms may not be freed. */
bool keep_syms;
/* If this is TRUE, the strings may not be freed. */
diff --git a/bfd/peicode.h b/bfd/peicode.h
index ad4f1963b9c..3a93e008799 100644
--- a/bfd/peicode.h
+++ b/bfd/peicode.h
@@ -1154,6 +1154,7 @@ pe_ILF_build_a_bfd (bfd * abfd,
obj_raw_syments (abfd) = vars.native_syms;
obj_raw_syment_count (abfd) = vars.sym_index;
+ obj_coff_keep_raw_syms (abfd) = true;
obj_coff_external_syms (abfd) = (void *) vars.esym_table;
obj_coff_keep_syms (abfd) = true;
@@ -3296,6 +3296,14 @@ _bfd_coff_free_cached_info (bfd *abfd)
These may have been set by pe_ILF_build_a_bfd() indicating
that the syms and strings pointers are not to be freed. */
_bfd_coff_free_symbols (abfd);
+
+ /* Free raw syms, and any other data bfd_alloc'd after raw syms
+ are read. */
+ if (obj_raw_syments (abfd))
+ bfd_release (abfd, obj_raw_syments (abfd));
+ obj_raw_syments (abfd) = NULL;
+ obj_symbols (abfd) = NULL;
+ obj_convert (abfd) = NULL;
}
return _bfd_generic_bfd_free_cached_info (abfd);
@@ -73,20 +73,16 @@ _bfd_new_bfd (void)
return NULL;
if (!bfd_lock ())
- return NULL;
+ goto loser;
nbfd->id = bfd_id_counter++;
if (!bfd_unlock ())
- {
- free (nbfd);
- return NULL;
- }
+ goto loser;
nbfd->memory = objalloc_create ();
if (nbfd->memory == NULL)
{
bfd_set_error (bfd_error_no_memory);
- free (nbfd);
- return NULL;
+ goto loser;
}
nbfd->arch_info = &bfd_default_arch_struct;
@@ -94,14 +90,16 @@ _bfd_new_bfd (void)
if (!bfd_hash_table_init_n (& nbfd->section_htab, bfd_section_hash_newfunc,
sizeof (struct section_hash_entry), 13))
{
- objalloc_free ((struct objalloc *) nbfd->memory);
- free (nbfd);
- return NULL;
+ objalloc_free (nbfd->memory);
+ goto loser;
}
nbfd->archive_plugin_fd = -1;
-
return nbfd;
+
+ loser:
+ free (nbfd);
+ return NULL;
}
static const struct bfd_iovec opncls_iovec;
@@ -153,13 +151,10 @@ _bfd_delete_bfd (bfd *abfd)
bfd_free_cached_info (abfd);
/* The target _bfd_free_cached_info may not have done anything.. */
+ if (abfd->section_htab.memory)
+ bfd_hash_table_free (&abfd->section_htab);
if (abfd->memory)
- {
- bfd_hash_table_free (&abfd->section_htab);
- objalloc_free ((struct objalloc *) abfd->memory);
- }
- else
- free ((char *) bfd_get_filename (abfd));
+ objalloc_free (abfd->memory);
#ifdef USE_MMAP
struct bfd_mmapped *mmapped, *next;
@@ -191,41 +186,15 @@ DESCRIPTION
bool
_bfd_free_cached_info (bfd *abfd)
{
- if (abfd->memory)
- {
- const char *filename = bfd_get_filename (abfd);
- if (filename)
- {
- /* We can't afford to lose the bfd filename when freeing
- abfd->memory, because that would kill the cache.c scheme
- of closing and reopening files in order to limit the
- number of open files. To reopen, you need the filename.
- And indeed _bfd_compute_and_write_armap calls
- _bfd_free_cached_info to free up space used by symbols
- and by check_format_matches. Which we want to continue
- doing to handle very large archives. Later the archive
- elements are copied, which might require reopening files.
- We also want to keep using objalloc memory for the
- filename since that allows the name to be updated
- without either leaking memory or implementing some sort
- of reference counted string for copies of the filename. */
- size_t len = strlen (filename) + 1;
- char *copy = bfd_malloc (len);
- if (copy == NULL)
- return false;
- memcpy (copy, filename, len);
- abfd->filename = copy;
- }
- bfd_hash_table_free (&abfd->section_htab);
- objalloc_free ((struct objalloc *) abfd->memory);
-
- abfd->sections = NULL;
- abfd->section_last = NULL;
- abfd->outsymbols = NULL;
- abfd->tdata.any = NULL;
- abfd->usrdata = NULL;
- abfd->memory = NULL;
- }
+ if (abfd->section_htab.memory)
+ bfd_hash_table_free (&abfd->section_htab);
+
+ abfd->sections = NULL;
+ abfd->section_last = NULL;
+ abfd->section_count = 0;
+ abfd->outsymbols = NULL;
+ abfd->tdata.any = NULL;
+ abfd->usrdata = NULL;
return true;
}
@@ -1043,10 +1012,18 @@ bfd_make_readable (bfd *abfd)
return false;
}
- if (! BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd)))
+ if (!BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd)))
return false;
- if (! BFD_SEND (abfd, _close_and_cleanup, (abfd)))
+ if (!BFD_SEND (abfd, _bfd_free_cached_info, (abfd)))
+ return false;
+
+ if (!BFD_SEND (abfd, _close_and_cleanup, (abfd)))
+ return false;
+
+ _bfd_free_cached_info (abfd);
+ if (!bfd_hash_table_init_n (&abfd->section_htab, bfd_section_hash_newfunc,
+ sizeof (struct section_hash_entry), 13))
return false;
abfd->arch_info = &bfd_default_arch_struct;
@@ -1057,20 +1034,17 @@ bfd_make_readable (bfd *abfd)
abfd->origin = 0;
abfd->opened_once = false;
abfd->output_has_begun = false;
- abfd->section_count = 0;
abfd->usrdata = NULL;
abfd->cacheable = false;
abfd->mtime_set = false;
abfd->target_defaulted = true;
abfd->direction = read_direction;
- abfd->sections = 0;
abfd->symcount = 0;
abfd->outsymbols = 0;
abfd->tdata.any = 0;
abfd->size = 0;
- bfd_section_list_clear (abfd);
bfd_check_format (abfd, bfd_object);
return true;