* [patch ld]: Close BFDs before linker-plugin's atexit routine is called
@ 2011-02-09 18:52 Kai Tietz
2011-02-13 10:40 ` Kai Tietz
2011-02-13 23:25 ` Alan Modra
0 siblings, 2 replies; 7+ messages in thread
From: Kai Tietz @ 2011-02-09 18:52 UTC (permalink / raw)
To: Binutils; +Cc: Nick Clifton
[-- Attachment #1: Type: text/plain, Size: 544 bytes --]
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.
Tested on x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?
Regards,
Kai
[-- Attachment #2: ld_release_bfd.txt --]
[-- Type: text/plain, Size: 1118 bytes --]
Index: src/ld/ldmain.c
===================================================================
--- src.orig/ld/ldmain.c 2011-01-18 19:49:25.000000000 +0100
+++ src/ld/ldmain.c 2011-02-09 18:49:38.148905600 +0100
@@ -180,6 +180,8 @@ remove_output (void)
{
if (link_info.output_bfd)
bfd_cache_close (link_info.output_bfd);
+ link_info.output_bfd = NULL;
+ bfd_cache_close_all ();
if (delete_output_file_on_failure)
unlink_if_ordinary (output_filename);
}
@@ -489,6 +491,10 @@ main (int argc, char **argv)
output_filename);
/* The file will be removed by remove_output. */
+ if (link_info.output_bfd)
+ bfd_cache_close (link_info.output_bfd);
+ link_info.output_bfd = NULL;
+ bfd_cache_close_all ();
xexit (1);
}
else
@@ -564,6 +570,10 @@ main (int argc, char **argv)
/* Prevent remove_output from doing anything, after a successful link. */
output_filename = NULL;
+ if (link_info.output_bfd)
+ bfd_cache_close (link_info.output_bfd);
+ link_info.output_bfd = NULL;
+ bfd_cache_close_all ();
xexit (0);
return 0;
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch ld]: Close BFDs before linker-plugin's atexit routine is called
2011-02-09 18:52 [patch ld]: Close BFDs before linker-plugin's atexit routine is called Kai Tietz
@ 2011-02-13 10:40 ` Kai Tietz
2011-02-13 23:25 ` Alan Modra
1 sibling, 0 replies; 7+ messages in thread
From: Kai Tietz @ 2011-02-13 10:40 UTC (permalink / raw)
To: Binutils; +Cc: Nick Clifton
PING.
2011/2/9 Kai Tietz <ktietz70@googlemail.com>:
> 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.
>
> Tested on x86_64-pc-linux-gnu and x86_64-w64-mingw32. Ok for apply?
>
> Regards,
> Kai
>
--
| (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch ld]: Close BFDs before linker-plugin's atexit routine is called
2011-02-09 18:52 [patch ld]: Close BFDs before linker-plugin's atexit routine is called Kai Tietz
2011-02-13 10:40 ` Kai Tietz
@ 2011-02-13 23:25 ` Alan Modra
2011-02-14 6:45 ` Kai Tietz
1 sibling, 1 reply; 7+ messages in thread
From: Alan Modra @ 2011-02-13 23:25 UTC (permalink / raw)
To: Kai Tietz; +Cc: Binutils, Nick Clifton
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.
The fact that you need to patch three places to fix one problem
says to me that this isn't the best fix.. Does a single
bfd_cache_close_all at the start of plugin_call_cleanup fix your
problem?
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch ld]: Close BFDs before linker-plugin's atexit routine is called
2011-02-13 23:25 ` Alan Modra
@ 2011-02-14 6:45 ` Kai Tietz
2011-02-14 9:22 ` Kai Tietz
0 siblings, 1 reply; 7+ messages in thread
From: Kai Tietz @ 2011-02-14 6:45 UTC (permalink / raw)
To: Kai Tietz, Binutils, Nick Clifton
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 ...
> Does a single
> bfd_cache_close_all at the start of plugin_call_cleanup fix your
> problem?
Yes, it does.
Regards,
Kai
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch ld]: Close BFDs before linker-plugin's atexit routine is called
2011-02-14 6:45 ` Kai Tietz
@ 2011-02-14 9:22 ` Kai Tietz
2011-02-14 9:51 ` Alan Modra
0 siblings, 1 reply; 7+ messages in thread
From: Kai Tietz @ 2011-02-14 9:22 UTC (permalink / raw)
To: Kai Tietz, Binutils, Nick Clifton, Alan Modra
[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]
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 ...
>
>> 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.
Changelog
2011-02-14 Kai Tietz
* ldmain.c (ld_close_bfds): New.
(remove_output): Call ld_close_bfds.
* plugin.c (plugin_call_cleanup): Likewise.
* ldmain.h (ld_close_bfds): Add prototype.
Tested for x86_64-pc-linux-gnu, and x86_64-w64-mingw32. Ok for apply?
Regards,
Kai
[-- Attachment #2: ld_bfd_close.txt --]
[-- Type: text/plain, Size: 1654 bytes --]
Index: src/ld/ldmain.c
===================================================================
--- src.orig/ld/ldmain.c 2011-02-10 09:47:59.000000000 +0100
+++ src/ld/ldmain.c 2011-02-14 10:13:39.624526400 +0100
@@ -173,13 +173,22 @@ static struct bfd_link_callbacks link_ca
struct bfd_link_info link_info;
\f
+void
+ld_close_bfds (void)
+{
+ if (link_info.output_bfd)
+ bfd_cache_close (link_info.output_bfd);
+ link_info.output_bfd = NULL;
+ bfd_cache_close_all ();
+}
+
static void
remove_output (void)
{
+ ld_close_bfds ();
+
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);
}
Index: src/ld/ldmain.h
===================================================================
--- src.orig/ld/ldmain.h 2009-12-14 10:33:59.000000000 +0100
+++ src/ld/ldmain.h 2011-02-14 10:14:04.499526400 +0100
@@ -45,5 +45,6 @@ extern int overflow_cutoff_limit;
extern void add_ysym (const char *);
extern void add_wrap (const char *);
extern void add_keepsyms_file (const char *);
+extern void ld_close_bfds (void);
#endif
Index: src/ld/plugin.c
===================================================================
--- src.orig/ld/plugin.c 2011-02-10 09:47:59.000000000 +0100
+++ src/ld/plugin.c 2011-02-14 10:15:38.608901400 +0100
@@ -824,6 +824,10 @@ static void
plugin_call_cleanup (void)
{
plugin_t *curplug = plugins_list;
+
+ /* Close all BFDs, so that unlink can operate on all targets. */
+ ld_close_bfds ();
+
while (curplug)
{
if (curplug->cleanup_handler && !curplug->cleanup_done)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch ld]: Close BFDs before linker-plugin's atexit routine is called
2011-02-14 9:22 ` Kai Tietz
@ 2011-02-14 9:51 ` Alan Modra
2011-02-14 10:01 ` Kai Tietz
0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2011-02-14 9:51 UTC (permalink / raw)
To: Kai Tietz; +Cc: Binutils, Nick Clifton
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;
\f
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [patch ld]: Close BFDs before linker-plugin's atexit routine is called
2011-02-14 9:51 ` Alan Modra
@ 2011-02-14 10:01 ` Kai Tietz
0 siblings, 0 replies; 7+ messages in thread
From: Kai Tietz @ 2011-02-14 10:01 UTC (permalink / raw)
To: Kai Tietz, Binutils, Nick Clifton
2011/2/14 Alan Modra <amodra@gmail.com>:
> 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
>
Yes, looks good and passes my tests.
Thanks,
Kai
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-14 10:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-09 18:52 [patch ld]: Close BFDs before linker-plugin's atexit routine is called Kai Tietz
2011-02-13 10:40 ` Kai Tietz
2011-02-13 23:25 ` Alan Modra
2011-02-14 6:45 ` Kai Tietz
2011-02-14 9:22 ` Kai Tietz
2011-02-14 9:51 ` Alan Modra
2011-02-14 10:01 ` Kai Tietz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).