This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA:] fix linker deallocation bug: link_info.hash deallocated too soon


On Fri, 9 May 2014, Alan Modra wrote:
> On Thu, May 08, 2014 at 07:27:02AM -0400, Hans-Peter Nilsson wrote:
> > At first I tried plainly moving the bfd_link_hash_table_free call into
> > ld_cleanup, but valgrind alerted me that bfd_link_hash_table_free
> > needs to deference the (deallocated) output_bfd.  The hash_table_free
> > function is "virtual"; it needs to dereference the BFD xvec "vtable".
>
> I think it still makes sense to use ld_cleanup, simply to avoid
> another atexit function.  Also, set output_bfd_hash_table_free_fn
> in open_output, after the hash table is created.  The patch is OK with
> those changes.

Thanks for the review!

I'd sooner worry about spreading this out over several files,
but here we go.  I'll commit this (and the testcases, excluded
this time) in the next 48h.  Tested as before.

	* ldlang.c (lang_finish): Don't call bfd_link_hash_table_free here.
	(output_bfd_hash_table_free_fn): New variable.
	(open_output): Save the _bfd_link_hash_table_free function for the
	output_bfd into output_bfd_hash_table_free_fn.
	* ldmain.c (ld_cleanup): If set, call output_bfd_hash_table_free_fn
	on link_info.hash.
	* ldlang.h (output_bfd_hash_table_free_fn): Declare.

diff --git a/ld/ldlang.c b/ld/ldlang.c
index d147ee0..8d1e3f7 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1237,7 +1237,6 @@ lang_init (void)
 void
 lang_finish (void)
 {
-  bfd_link_hash_table_free (link_info.output_bfd, link_info.hash);
   bfd_hash_table_free (&lang_definedness_table);
   output_section_statement_table_free ();
 }
@@ -3073,6 +3072,9 @@ lang_get_output_target (void)
   return default_target;
 }

+/* Stashed function to free link_info.hash; see open_output.  */
+void (*output_bfd_hash_table_free_fn) (struct bfd_link_hash_table *);
+
 /* Open the output file.  */

 static void
@@ -3152,6 +3154,18 @@ open_output (const char *name)
   if (link_info.hash == NULL)
     einfo (_("%P%F: can not create hash table: %E\n"));

+  /* We want to please memory leak checkers by deleting link_info.hash.
+     We can't do it in lang_finish, as a bfd target may hold references to
+     symbols in this table and use them when their _bfd_write_contents
+     function is invoked, as part of bfd_close on the output_bfd.  But,
+     output_bfd is deallocated at bfd_close, so we can't refer to
+     output_bfd after that time, and dereferencing it is needed to call
+     "bfd_link_hash_table_free".  Smash this dependency deadlock and grab
+     the function pointer; arrange to call it on link_info.hash in
+     ld_cleanup.  */
+  output_bfd_hash_table_free_fn
+    = link_info.output_bfd->xvec->_bfd_link_hash_table_free;
+
   bfd_set_gp_size (link_info.output_bfd, g_switch_value);
 }

diff --git a/ld/ldlang.h b/ld/ldlang.h
index aacd5dc..47cc4df 100644
--- a/ld/ldlang.h
+++ b/ld/ldlang.h
@@ -518,6 +518,8 @@ extern lang_statement_list_type input_file_chain;
 extern int lang_statement_iteration;
 extern struct asneeded_minfo **asneeded_list_tail;

+extern void (*output_bfd_hash_table_free_fn) (struct bfd_link_hash_table *);
+
 extern void lang_init
   (void);
 extern void lang_finish
diff --git a/ld/ldmain.c b/ld/ldmain.c
index 4c7ea68..2d987b8 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -171,6 +171,10 @@ ld_cleanup (void)
 #endif
   if (output_filename && delete_output_file_on_failure)
     unlink_if_ordinary (output_filename);
+
+  /* See open_output in ldlang.c.  */
+  if (output_bfd_hash_table_free_fn != NULL)
+    (*output_bfd_hash_table_free_fn) (link_info.hash);
 }

 /* If there's a BFD assertion, we'll notice and exit with an error

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]