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: [patch ld]: Close BFDs before linker-plugin's atexit routine is called


On Mon, Feb 14, 2011 at 10:21:58AM +0100, Kai Tietz wrote:
> 2011/2/14 Kai Tietz <ktietz70@googlemail.com>:
> > 2011/2/14 Alan Modra <amodra@gmail.com>:
> >> On Wed, Feb 09, 2011 at 07:51:58PM +0100, Kai Tietz wrote:
> >>> Hello,
> >>>
> >>> This is patch addresses the unlink call when lto linker-plugin is
> >>> used. ?As windows
> >>> native doesn't support to unlink still opened files, it fails to do so
> >>> as file-descriptors
> >>> of bfds aren't closed before atexit routine of plugin gets called.
> >>>
> >>> 2011-02-09 ?Kai Tietz
> >>>
> >>> ? ? ? * ldmain.c (remove_output): Set output_bfd
> >>> ? ? ? of link_info to nil and close all cached bfds.
> >>> ? ? ? (main): Close output_bfd of link_info and set
> >>> ? ? ? it to nil. Additionally close all cached bfds.
> >
> > Hi Alan,
> >
> >> The fact that you need to patch three places to fix one problem
> >> says to me that this isn't the best fix..
> > Well, here I am not that sure about. I thought about fixing it just
> > within the atexit-handler of plugin, but well, next one trying to
> > opearate on files in a similar way, will have the same issues again. I
> > think it is simply bad style to exit a program without even try to
> > cleanup ?opened file descriptors and used memory. This could even help
> > to use tools like valgrind on ld ...

Yes, it's true that closing the files really belongs in ld itself.

> >> ?Does a single
> >> bfd_cache_close_all at the start of plugin_call_cleanup fix your
> >> problem?
> > Yes, it does.
> 
> I reworked patch a bit. It takes care that in ldmain's local
> atexit-handler BFDs are closed, and that for case of plugin the bfds
> getting closed before it tries to unlink
> files.

Better, but this is what I'm about to commit.

	* ldmain.c (remove_output): Rename to..
	(ld_cleanup): ..this. Call bfd_cache_close_all and plugin_call_cleanup.
	(main): Adjust.
	* plugin.c (plugin_call_cleanup): Make global.
	(plugin_load_plugins): Don't register plugin_call_cleanup with xatexit.
	* plugin.h (plugin_call_cleanup): Declare.

Index: ld/ldmain.c
===================================================================
RCS file: /cvs/src/src/ld/ldmain.c,v
retrieving revision 1.148
diff -u -p -r1.148 ldmain.c
--- ld/ldmain.c	14 Jan 2011 12:37:17 -0000	1.148
+++ ld/ldmain.c	14 Feb 2011 07:57:25 -0000
@@ -174,15 +174,14 @@ static struct bfd_link_callbacks link_ca
 struct bfd_link_info link_info;
 
 static void
-remove_output (void)
+ld_cleanup (void)
 {
-  if (output_filename)
-    {
-      if (link_info.output_bfd)
-	bfd_cache_close (link_info.output_bfd);
-      if (delete_output_file_on_failure)
-	unlink_if_ordinary (output_filename);
-    }
+  bfd_cache_close_all ();
+#ifdef ENABLE_PLUGINS
+  plugin_call_cleanup ();
+#endif
+  if (output_filename && delete_output_file_on_failure)
+    unlink_if_ordinary (output_filename);
 }
 
 int
@@ -211,7 +210,7 @@ main (int argc, char **argv)
 
   bfd_set_error_program_name (program_name);
 
-  xatexit (remove_output);
+  xatexit (ld_cleanup);
 
   /* Set up the sysroot directory.  */
   ld_sysroot = get_sysroot (argc, argv);
Index: ld/plugin.c
===================================================================
RCS file: /cvs/src/src/ld/plugin.c,v
retrieving revision 1.22
diff -u -p -r1.22 plugin.c
--- ld/plugin.c	22 Jan 2011 10:16:29 -0000	1.22
+++ ld/plugin.c	14 Feb 2011 07:57:26 -0000
@@ -108,9 +108,6 @@ static bfd_boolean no_more_claiming = FA
    TRUE is returned from the hook.  */
 static bfd_boolean plugin_cached_allow_multiple_defs = FALSE;
 
-/* Call 'cleanup' hook for all plugins at exit.  */
-static void plugin_call_cleanup (void);
-
 /* List of tags to set in the constant leading part of the tv array. */
 static const enum ld_plugin_tag tv_header_tags[] =
 {
@@ -721,8 +723,6 @@ plugin_load_plugins (void)
   if (!curplug)
     return 0;
 
-  xatexit (plugin_call_cleanup);
-
   /* First pass over plugins to find max # args needed so that we
      can size and allocate the tv array.  */
   while (curplug)
@@ -820,7 +820,7 @@ plugin_call_all_symbols_read (void)
 }
 
 /* Call 'cleanup' hook for all plugins at exit.  */
-static void
+void
 plugin_call_cleanup (void)
 {
   plugin_t *curplug = plugins_list;
Index: ld/plugin.h
===================================================================
RCS file: /cvs/src/src/ld/plugin.h,v
retrieving revision 1.5
diff -u -p -r1.5 plugin.h
--- ld/plugin.h	6 Dec 2010 12:44:51 -0000	1.5
+++ ld/plugin.h	14 Feb 2011 07:57:26 -0000
@@ -50,6 +50,9 @@ extern int plugin_call_claim_file (const
 /* Call 'all symbols read' hook for all plugins.  */
 extern int plugin_call_all_symbols_read (void);
 
+/* Call 'cleanup' hook for all plugins at exit.  */
+extern void plugin_call_cleanup (void);
+
 /* Generate a dummy BFD to represent an IR file, for any callers of
    plugin_call_claim_file to use as the handle in the ld_plugin_input_file
    struct that they build to pass in.  The BFD is initially writable, so

-- 
Alan Modra
Australia Development Lab, IBM


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