* PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
@ 2015-01-28 18:51 H.J. Lu
2015-01-28 19:57 ` Richard Biener
0 siblings, 1 reply; 17+ messages in thread
From: H.J. Lu @ 2015-01-28 18:51 UTC (permalink / raw)
To: gcc-patches
Hi,
This patch makes claim_file_handler to call release_input_file after it
finishes processing input file. OK for trunk?
Thanks.
H.J.
---
diff --git a/lto-plugin/ChangeLog b/lto-plugin/ChangeLog
index e8ec05b..c0eae24 100644
--- a/lto-plugin/ChangeLog
+++ b/lto-plugin/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR lto/64837
+ * lto-plugin.c (release_input_file): New.
+ (claim_file_handler): Call release_input_file.
+ (onload): Set release_input_file.
+
2014-12-09 Ilya Verbin <ilya.verbin@intel.com>
* lto-plugin.c (offload_files, num_offload_files): New static variables.
diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
index 8d957402..8e0a657 100644
--- a/lto-plugin/lto-plugin.c
+++ b/lto-plugin/lto-plugin.c
@@ -145,6 +145,7 @@ static ld_plugin_register_all_symbols_read register_all_symbols_read;
static ld_plugin_get_symbols get_symbols, get_symbols_v2;
static ld_plugin_register_cleanup register_cleanup;
static ld_plugin_add_input_file add_input_file;
+static ld_plugin_release_input_file release_input_file;
static ld_plugin_add_input_library add_input_library;
static ld_plugin_message message;
static ld_plugin_add_symbols add_symbols;
@@ -1006,6 +1007,8 @@ claim_file_handler (const struct ld_plugin_input_file *file, int *claimed)
if (obj.objfile)
simple_object_release_read (obj.objfile);
+ release_input_file (file);
+
return LDPS_OK;
}
@@ -1091,6 +1094,9 @@ onload (struct ld_plugin_tv *tv)
case LDPT_ADD_INPUT_FILE:
add_input_file = p->tv_u.tv_add_input_file;
break;
+ case LDPT_RELEASE_INPUT_FILE:
+ release_input_file = p->tv_u.tv_release_input_file;
+ break;
case LDPT_ADD_INPUT_LIBRARY:
add_input_library = p->tv_u.tv_add_input_library;
break;
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-28 18:51 PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file H.J. Lu
@ 2015-01-28 19:57 ` Richard Biener
2015-01-28 20:05 ` H.J. Lu
0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2015-01-28 19:57 UTC (permalink / raw)
To: H.J. Lu, H.J. Lu, gcc-patches
On January 28, 2015 7:12:43 PM CET, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>Hi,
>
>This patch makes claim_file_handler to call release_input_file after it
>finishes processing input file. OK for trunk?
OK. How did you test this?
Thanks,
Richard.
>Thanks.
>
>
>H.J.
>---
>diff --git a/lto-plugin/ChangeLog b/lto-plugin/ChangeLog
>index e8ec05b..c0eae24 100644
>--- a/lto-plugin/ChangeLog
>+++ b/lto-plugin/ChangeLog
>@@ -1,3 +1,10 @@
>+2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>+
>+ PR lto/64837
>+ * lto-plugin.c (release_input_file): New.
>+ (claim_file_handler): Call release_input_file.
>+ (onload): Set release_input_file.
>+
> 2014-12-09 Ilya Verbin <ilya.verbin@intel.com>
>
> * lto-plugin.c (offload_files, num_offload_files): New static
>variables.
>diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
>index 8d957402..8e0a657 100644
>--- a/lto-plugin/lto-plugin.c
>+++ b/lto-plugin/lto-plugin.c
>@@ -145,6 +145,7 @@ static ld_plugin_register_all_symbols_read
>register_all_symbols_read;
> static ld_plugin_get_symbols get_symbols, get_symbols_v2;
> static ld_plugin_register_cleanup register_cleanup;
> static ld_plugin_add_input_file add_input_file;
>+static ld_plugin_release_input_file release_input_file;
> static ld_plugin_add_input_library add_input_library;
> static ld_plugin_message message;
> static ld_plugin_add_symbols add_symbols;
>@@ -1006,6 +1007,8 @@ claim_file_handler (const struct
>ld_plugin_input_file *file, int *claimed)
> if (obj.objfile)
> simple_object_release_read (obj.objfile);
>
>+ release_input_file (file);
>+
> return LDPS_OK;
> }
>
>@@ -1091,6 +1094,9 @@ onload (struct ld_plugin_tv *tv)
> case LDPT_ADD_INPUT_FILE:
> add_input_file = p->tv_u.tv_add_input_file;
> break;
>+ case LDPT_RELEASE_INPUT_FILE:
>+ release_input_file = p->tv_u.tv_release_input_file;
>+ break;
> case LDPT_ADD_INPUT_LIBRARY:
> add_input_library = p->tv_u.tv_add_input_library;
> break;
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-28 19:57 ` Richard Biener
@ 2015-01-28 20:05 ` H.J. Lu
2015-01-28 20:17 ` Cary Coutant
2015-01-29 5:07 ` H.J. Lu
0 siblings, 2 replies; 17+ messages in thread
From: H.J. Lu @ 2015-01-28 20:05 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Wed, Jan 28, 2015 at 11:19 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On January 28, 2015 7:12:43 PM CET, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>>Hi,
>>
>>This patch makes claim_file_handler to call release_input_file after it
>>finishes processing input file. OK for trunk?
>
> OK. How did you test this?
I did normal bootstrap and "make check" on Linux/x86-64.
I also run ld.bfd and ld.gold by hand to verify that release_input_file
is called.
H.J.
> Thanks,
> Richard.
>
>
>>Thanks.
>>
>>
>>H.J.
>>---
>>diff --git a/lto-plugin/ChangeLog b/lto-plugin/ChangeLog
>>index e8ec05b..c0eae24 100644
>>--- a/lto-plugin/ChangeLog
>>+++ b/lto-plugin/ChangeLog
>>@@ -1,3 +1,10 @@
>>+2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>>+
>>+ PR lto/64837
>>+ * lto-plugin.c (release_input_file): New.
>>+ (claim_file_handler): Call release_input_file.
>>+ (onload): Set release_input_file.
>>+
>> 2014-12-09 Ilya Verbin <ilya.verbin@intel.com>
>>
>> * lto-plugin.c (offload_files, num_offload_files): New static
>>variables.
>>diff --git a/lto-plugin/lto-plugin.c b/lto-plugin/lto-plugin.c
>>index 8d957402..8e0a657 100644
>>--- a/lto-plugin/lto-plugin.c
>>+++ b/lto-plugin/lto-plugin.c
>>@@ -145,6 +145,7 @@ static ld_plugin_register_all_symbols_read
>>register_all_symbols_read;
>> static ld_plugin_get_symbols get_symbols, get_symbols_v2;
>> static ld_plugin_register_cleanup register_cleanup;
>> static ld_plugin_add_input_file add_input_file;
>>+static ld_plugin_release_input_file release_input_file;
>> static ld_plugin_add_input_library add_input_library;
>> static ld_plugin_message message;
>> static ld_plugin_add_symbols add_symbols;
>>@@ -1006,6 +1007,8 @@ claim_file_handler (const struct
>>ld_plugin_input_file *file, int *claimed)
>> if (obj.objfile)
>> simple_object_release_read (obj.objfile);
>>
>>+ release_input_file (file);
>>+
>> return LDPS_OK;
>> }
>>
>>@@ -1091,6 +1094,9 @@ onload (struct ld_plugin_tv *tv)
>> case LDPT_ADD_INPUT_FILE:
>> add_input_file = p->tv_u.tv_add_input_file;
>> break;
>>+ case LDPT_RELEASE_INPUT_FILE:
>>+ release_input_file = p->tv_u.tv_release_input_file;
>>+ break;
>> case LDPT_ADD_INPUT_LIBRARY:
>> add_input_library = p->tv_u.tv_add_input_library;
>> break;
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-28 20:05 ` H.J. Lu
@ 2015-01-28 20:17 ` Cary Coutant
2015-01-28 20:18 ` H.J. Lu
2015-01-29 5:07 ` H.J. Lu
1 sibling, 1 reply; 17+ messages in thread
From: Cary Coutant @ 2015-01-28 20:17 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Biener, GCC Patches
>>>This patch makes claim_file_handler to call release_input_file after it
>>>finishes processing input file. OK for trunk?
>>
>> OK. How did you test this?
>
> I did normal bootstrap and "make check" on Linux/x86-64.
> I also run ld.bfd and ld.gold by hand to verify that release_input_file
> is called.
But release_input_file is only supposed to be used after calling
get_input_file from the all_symbols_read handler. It's not needed in
the claim_file handler. If gold isn't freeing the file descriptor
properly in that case, it's a bug entirely within gold (I think). I'm
looking at it.
-cary
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-28 20:17 ` Cary Coutant
@ 2015-01-28 20:18 ` H.J. Lu
0 siblings, 0 replies; 17+ messages in thread
From: H.J. Lu @ 2015-01-28 20:18 UTC (permalink / raw)
To: Cary Coutant; +Cc: Richard Biener, GCC Patches
On Wed, Jan 28, 2015 at 11:44 AM, Cary Coutant <ccoutant@google.com> wrote:
>>>>This patch makes claim_file_handler to call release_input_file after it
>>>>finishes processing input file. OK for trunk?
>>>
>>> OK. How did you test this?
>>
>> I did normal bootstrap and "make check" on Linux/x86-64.
>> I also run ld.bfd and ld.gold by hand to verify that release_input_file
>> is called.
>
> But release_input_file is only supposed to be used after calling
> get_input_file from the all_symbols_read handler. It's not needed in
> the claim_file handler. If gold isn't freeing the file descriptor
> properly in that case, it's a bug entirely within gold (I think). I'm
> looking at it.
release_input_file should be allowed in the claim_file handler
since GCC plugin doesn't use get_input_file at all.
--
H.J.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-28 20:05 ` H.J. Lu
2015-01-28 20:17 ` Cary Coutant
@ 2015-01-29 5:07 ` H.J. Lu
2015-02-04 4:49 ` Cary Coutant
2015-02-05 16:43 ` H.J. Lu
1 sibling, 2 replies; 17+ messages in thread
From: H.J. Lu @ 2015-01-29 5:07 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Wed, Jan 28, 2015 at 11:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 28, 2015 at 11:19 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On January 28, 2015 7:12:43 PM CET, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>>>Hi,
>>>
>>>This patch makes claim_file_handler to call release_input_file after it
>>>finishes processing input file. OK for trunk?
>>
>> OK. How did you test this?
>
> I did normal bootstrap and "make check" on Linux/x86-64.
> I also run ld.bfd and ld.gold by hand to verify that release_input_file
> is called.
>
This is needed for LTO build. ar/nm/ranlib don't provide
release_input_file. I checked it in as an obvious fix.
--
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 220212)
+++ ChangeLog (working copy)
@@ -1,5 +1,10 @@
2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
+ * lto-plugin.c (claim_file_handler): Call release_input_file only
+ if it is not NULL.
+
+2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
+
PR lto/64837
* lto-plugin.c (release_input_file): New.
(claim_file_handler): Call release_input_file.
Index: lto-plugin.c
===================================================================
--- lto-plugin.c (revision 220212)
+++ lto-plugin.c (working copy)
@@ -1007,7 +1007,8 @@ claim_file_handler (const struct ld_plug
if (obj.objfile)
simple_object_release_read (obj.objfile);
- release_input_file (file);
+ if (release_input_file)
+ release_input_file (file);
return LDPS_OK;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-29 5:07 ` H.J. Lu
@ 2015-02-04 4:49 ` Cary Coutant
2015-02-04 8:57 ` Richard Biener
2015-02-05 16:43 ` H.J. Lu
1 sibling, 1 reply; 17+ messages in thread
From: Cary Coutant @ 2015-02-04 4:49 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Biener, GCC Patches
The plugin is not supposed to call release_input_file from the
claim_file handler. That interface is only for releasing a file
descriptor obtained via get_input_file during the all_symbols_read
callback. When the linker calls the claim_file handler, the file
descriptor is open, and the plugin is required to leave it open; the
linker manages the file descriptor at that point. The
get_input_file/release_input_file pair of interfaces was added later,
for the benefit of another (non-LTO) plugin (although I think the LLVM
LTO plugin does use that pair during the all_symbols_read callback).
This is described here:
https://gcc.gnu.org/wiki/whopr/driver
If you're going to insist on calling the release_input_file API from
the claim_file handler, I'm going to have to fix gold to ignore the
call to avoid a premature unlock of the object file.
-cary
On Wed, Jan 28, 2015 at 4:02 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 28, 2015 at 11:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jan 28, 2015 at 11:19 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On January 28, 2015 7:12:43 PM CET, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>>>>Hi,
>>>>
>>>>This patch makes claim_file_handler to call release_input_file after it
>>>>finishes processing input file. OK for trunk?
>>>
>>> OK. How did you test this?
>>
>> I did normal bootstrap and "make check" on Linux/x86-64.
>> I also run ld.bfd and ld.gold by hand to verify that release_input_file
>> is called.
>>
>
> This is needed for LTO build. ar/nm/ranlib don't provide
> release_input_file. I checked it in as an obvious fix.
>
> --
> H.J.
> ---
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 220212)
> +++ ChangeLog (working copy)
> @@ -1,5 +1,10 @@
> 2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>
> + * lto-plugin.c (claim_file_handler): Call release_input_file only
> + if it is not NULL.
> +
> +2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
> +
> PR lto/64837
> * lto-plugin.c (release_input_file): New.
> (claim_file_handler): Call release_input_file.
> Index: lto-plugin.c
> ===================================================================
> --- lto-plugin.c (revision 220212)
> +++ lto-plugin.c (working copy)
> @@ -1007,7 +1007,8 @@ claim_file_handler (const struct ld_plug
> if (obj.objfile)
> simple_object_release_read (obj.objfile);
>
> - release_input_file (file);
> + if (release_input_file)
> + release_input_file (file);
>
> return LDPS_OK;
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-04 4:49 ` Cary Coutant
@ 2015-02-04 8:57 ` Richard Biener
2015-02-04 15:35 ` Cary Coutant
0 siblings, 1 reply; 17+ messages in thread
From: Richard Biener @ 2015-02-04 8:57 UTC (permalink / raw)
To: Cary Coutant, H.J. Lu; +Cc: GCC Patches
On February 4, 2015 5:49:13 AM CET, Cary Coutant <ccoutant@google.com> wrote:
>The plugin is not supposed to call release_input_file from the
>claim_file handler. That interface is only for releasing a file
>descriptor obtained via get_input_file during the all_symbols_read
>callback. When the linker calls the claim_file handler, the file
>descriptor is open, and the plugin is required to leave it open; the
>linker manages the file descriptor at that point. The
>get_input_file/release_input_file pair of interfaces was added later,
>for the benefit of another (non-LTO) plugin (although I think the LLVM
>LTO plugin does use that pair during the all_symbols_read callback).
>
>This is described here:
>
> https://gcc.gnu.org/wiki/whopr/driver
>
>If you're going to insist on calling the release_input_file API from
>the claim_file handler, I'm going to have to fix gold to ignore the
>call to avoid a premature unlock of the object file.
What's the proper solution for not leaking those filedescriptors?
Richard.
>-cary
>
>
>
>On Wed, Jan 28, 2015 at 4:02 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jan 28, 2015 at 11:37 AM, H.J. Lu <hjl.tools@gmail.com>
>wrote:
>>> On Wed, Jan 28, 2015 at 11:19 AM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On January 28, 2015 7:12:43 PM CET, "H.J. Lu"
><hongjiu.lu@intel.com> wrote:
>>>>>Hi,
>>>>>
>>>>>This patch makes claim_file_handler to call release_input_file
>after it
>>>>>finishes processing input file. OK for trunk?
>>>>
>>>> OK. How did you test this?
>>>
>>> I did normal bootstrap and "make check" on Linux/x86-64.
>>> I also run ld.bfd and ld.gold by hand to verify that
>release_input_file
>>> is called.
>>>
>>
>> This is needed for LTO build. ar/nm/ranlib don't provide
>> release_input_file. I checked it in as an obvious fix.
>>
>> --
>> H.J.
>> ---
>> Index: ChangeLog
>> ===================================================================
>> --- ChangeLog (revision 220212)
>> +++ ChangeLog (working copy)
>> @@ -1,5 +1,10 @@
>> 2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>>
>> + * lto-plugin.c (claim_file_handler): Call release_input_file only
>> + if it is not NULL.
>> +
>> +2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>> +
>> PR lto/64837
>> * lto-plugin.c (release_input_file): New.
>> (claim_file_handler): Call release_input_file.
>> Index: lto-plugin.c
>> ===================================================================
>> --- lto-plugin.c (revision 220212)
>> +++ lto-plugin.c (working copy)
>> @@ -1007,7 +1007,8 @@ claim_file_handler (const struct ld_plug
>> if (obj.objfile)
>> simple_object_release_read (obj.objfile);
>>
>> - release_input_file (file);
>> + if (release_input_file)
>> + release_input_file (file);
>>
>> return LDPS_OK;
>> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-04 8:57 ` Richard Biener
@ 2015-02-04 15:35 ` Cary Coutant
2015-02-04 15:52 ` H.J. Lu
0 siblings, 1 reply; 17+ messages in thread
From: Cary Coutant @ 2015-02-04 15:35 UTC (permalink / raw)
To: Richard Biener; +Cc: H.J. Lu, GCC Patches
>>If you're going to insist on calling the release_input_file API from
>>the claim_file handler, I'm going to have to fix gold to ignore the
>>call to avoid a premature unlock of the object file.
>
> What's the proper solution for not leaking those filedescriptors?
There was a bug in gold where it wasn't unlocking external members of
thin archives that got claimed by the plugin. See PR 15660:
https://sourceware.org/bugzilla/show_bug.cgi?id=15660
-cary
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-04 15:35 ` Cary Coutant
@ 2015-02-04 15:52 ` H.J. Lu
0 siblings, 0 replies; 17+ messages in thread
From: H.J. Lu @ 2015-02-04 15:52 UTC (permalink / raw)
To: Cary Coutant; +Cc: Richard Biener, GCC Patches
On Wed, Feb 4, 2015 at 7:35 AM, Cary Coutant <ccoutant@google.com> wrote:
>>>If you're going to insist on calling the release_input_file API from
>>>the claim_file handler, I'm going to have to fix gold to ignore the
>>>call to avoid a premature unlock of the object file.
>>
>> What's the proper solution for not leaking those filedescriptors?
>
> There was a bug in gold where it wasn't unlocking external members of
> thin archives that got claimed by the plugin. See PR 15660:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=15660
>
FWIW, ld doesn't have the file descriptor leak since it does
/* fd belongs to us, not the plugin; but we don't need it. */
close (file->fd);
after calling claim_file_handler. It only works with GCC plug-in
library. I going to change it:
https://sourceware.org/ml/binutils/2015-02/msg00001.html
--
H.J.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-01-29 5:07 ` H.J. Lu
2015-02-04 4:49 ` Cary Coutant
@ 2015-02-05 16:43 ` H.J. Lu
2015-02-05 20:58 ` H.J. Lu
1 sibling, 1 reply; 17+ messages in thread
From: H.J. Lu @ 2015-02-05 16:43 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Wed, Jan 28, 2015 at 4:02 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 28, 2015 at 11:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jan 28, 2015 at 11:19 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> On January 28, 2015 7:12:43 PM CET, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>>>>Hi,
>>>>
>>>>This patch makes claim_file_handler to call release_input_file after it
>>>>finishes processing input file. OK for trunk?
>>>
>>> OK. How did you test this?
>>
>> I did normal bootstrap and "make check" on Linux/x86-64.
>> I also run ld.bfd and ld.gold by hand to verify that release_input_file
>> is called.
>>
>
> This is needed for LTO build. ar/nm/ranlib don't provide
> release_input_file. I checked it in as an obvious fix.
>
> --
> H.J.
> ---
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 220212)
> +++ ChangeLog (working copy)
> @@ -1,5 +1,10 @@
> 2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>
> + * lto-plugin.c (claim_file_handler): Call release_input_file only
> + if it is not NULL.
> +
> +2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
> +
> PR lto/64837
> * lto-plugin.c (release_input_file): New.
> (claim_file_handler): Call release_input_file.
> Index: lto-plugin.c
> ===================================================================
> --- lto-plugin.c (revision 220212)
> +++ lto-plugin.c (working copy)
> @@ -1007,7 +1007,8 @@ claim_file_handler (const struct ld_plug
> if (obj.objfile)
> simple_object_release_read (obj.objfile);
>
> - release_input_file (file);
> + if (release_input_file)
> + release_input_file (file);
>
> return LDPS_OK;
> }
We should call release_input_file only if file is claimed. Otherwise,
ld will close file descriptor of non-IR file. I checked it in as obvious fix.
--
H.J.
---
Index: ChangeLog
===================================================================
--- ChangeLog (revision 220454)
+++ ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2015-02-05 H.J. Lu <hongjiu.lu@intel.com>
+
+ * lto-plugin.c (claim_file_handler): Call release_input_file only
+ if file is claimed.
+
2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
* lto-plugin.c (claim_file_handler): Call release_input_file only
Index: lto-plugin.c
===================================================================
--- lto-plugin.c (revision 220454)
+++ lto-plugin.c (working copy)
@@ -998,6 +998,9 @@ claim_file_handler (const struct ld_plug
*claimed = 1;
+ if (release_input_file)
+ release_input_file (file);
+
goto cleanup;
err:
@@ -1007,9 +1010,6 @@ claim_file_handler (const struct ld_plug
if (obj.objfile)
simple_object_release_read (obj.objfile);
- if (release_input_file)
- release_input_file (file);
-
return LDPS_OK;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-05 16:43 ` H.J. Lu
@ 2015-02-05 20:58 ` H.J. Lu
2015-02-06 12:56 ` Markus Trippelsdorf
0 siblings, 1 reply; 17+ messages in thread
From: H.J. Lu @ 2015-02-05 20:58 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Thu, Feb 5, 2015 at 8:42 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 28, 2015 at 4:02 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jan 28, 2015 at 11:37 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Jan 28, 2015 at 11:19 AM, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> On January 28, 2015 7:12:43 PM CET, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>>>>>Hi,
>>>>>
>>>>>This patch makes claim_file_handler to call release_input_file after it
>>>>>finishes processing input file. OK for trunk?
>>>>
>>>> OK. How did you test this?
>>>
>>> I did normal bootstrap and "make check" on Linux/x86-64.
>>> I also run ld.bfd and ld.gold by hand to verify that release_input_file
>>> is called.
>>>
>>
>> This is needed for LTO build. ar/nm/ranlib don't provide
>> release_input_file. I checked it in as an obvious fix.
>>
>> --
>> H.J.
>> ---
>> Index: ChangeLog
>> ===================================================================
>> --- ChangeLog (revision 220212)
>> +++ ChangeLog (working copy)
>> @@ -1,5 +1,10 @@
>> 2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>>
>> + * lto-plugin.c (claim_file_handler): Call release_input_file only
>> + if it is not NULL.
>> +
>> +2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>> +
>> PR lto/64837
>> * lto-plugin.c (release_input_file): New.
>> (claim_file_handler): Call release_input_file.
>> Index: lto-plugin.c
>> ===================================================================
>> --- lto-plugin.c (revision 220212)
>> +++ lto-plugin.c (working copy)
>> @@ -1007,7 +1007,8 @@ claim_file_handler (const struct ld_plug
>> if (obj.objfile)
>> simple_object_release_read (obj.objfile);
>>
>> - release_input_file (file);
>> + if (release_input_file)
>> + release_input_file (file);
>>
>> return LDPS_OK;
>> }
>
> We should call release_input_file only if file is claimed. Otherwise,
> ld will close file descriptor of non-IR file. I checked it in as obvious fix.
>
> --
> H.J.
> ---
> Index: ChangeLog
> ===================================================================
> --- ChangeLog (revision 220454)
> +++ ChangeLog (working copy)
> @@ -1,3 +1,8 @@
> +2015-02-05 H.J. Lu <hongjiu.lu@intel.com>
> +
> + * lto-plugin.c (claim_file_handler): Call release_input_file only
> + if file is claimed.
> +
> 2015-01-28 H.J. Lu <hongjiu.lu@intel.com>
>
> * lto-plugin.c (claim_file_handler): Call release_input_file only
> Index: lto-plugin.c
> ===================================================================
> --- lto-plugin.c (revision 220454)
> +++ lto-plugin.c (working copy)
> @@ -998,6 +998,9 @@ claim_file_handler (const struct ld_plug
>
> *claimed = 1;
>
> + if (release_input_file)
> + release_input_file (file);
> +
> goto cleanup;
>
> err:
> @@ -1007,9 +1010,6 @@ claim_file_handler (const struct ld_plug
> if (obj.objfile)
> simple_object_release_read (obj.objfile);
>
> - if (release_input_file)
> - release_input_file (file);
> -
> return LDPS_OK;
> }
We should pass handle, not file, to release_input_file.
I checked it in as an obvious fix.
--
H.J.
--
Index: ChangeLog
===================================================================
--- ChangeLog (revision 220455)
+++ ChangeLog (working copy)
@@ -1,5 +1,10 @@
2015-02-05 H.J. Lu <hongjiu.lu@intel.com>
+ * lto-plugin.c (claim_file_handler): Pass handle to
+ release_input_file.
+
+2015-02-05 H.J. Lu <hongjiu.lu@intel.com>
+
* lto-plugin.c (claim_file_handler): Call release_input_file only
if file is claimed.
Index: lto-plugin.c
===================================================================
--- lto-plugin.c (revision 220455)
+++ lto-plugin.c (working copy)
@@ -999,7 +999,7 @@ claim_file_handler (const struct ld_plug
*claimed = 1;
if (release_input_file)
- release_input_file (file);
+ release_input_file (file->handle);
goto cleanup;
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-05 20:58 ` H.J. Lu
@ 2015-02-06 12:56 ` Markus Trippelsdorf
2015-02-06 13:11 ` H.J. Lu
0 siblings, 1 reply; 17+ messages in thread
From: Markus Trippelsdorf @ 2015-02-06 12:56 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Biener, GCC Patches
On 2015.02.05 at 12:57 -0800, H.J. Lu wrote:
>
> We should pass handle, not file, to release_input_file.
> I checked it in as an obvious fix.
This commit causes:
% echo "int main () {}" | gcc -fuse-ld=gold -flto -x c++ -
ld.gold: internal error in remove_writer, at token.h:132
collect2: error: ld returned 1 exit status
--
Markus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-06 12:56 ` Markus Trippelsdorf
@ 2015-02-06 13:11 ` H.J. Lu
2015-02-06 13:19 ` Markus Trippelsdorf
0 siblings, 1 reply; 17+ messages in thread
From: H.J. Lu @ 2015-02-06 13:11 UTC (permalink / raw)
To: Markus Trippelsdorf; +Cc: Richard Biener, GCC Patches
On Fri, Feb 6, 2015 at 4:56 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2015.02.05 at 12:57 -0800, H.J. Lu wrote:
>>
>> We should pass handle, not file, to release_input_file.
>> I checked it in as an obvious fix.
>
> This commit causes:
>
> % echo "int main () {}" | gcc -fuse-ld=gold -flto -x c++ -
> ld.gold: internal error in remove_writer, at token.h:132
> collect2: error: ld returned 1 exit status
>
ld.bfd closes fd without release_input_file being called
and ld.gold leaks file descriptors if release_input_file
isn't called. We can either fix gold:
https://sourceware.org/bugzilla/show_bug.cgi?id=17896
or revert the PR lto/64837 fix.
--
H.J.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-06 13:11 ` H.J. Lu
@ 2015-02-06 13:19 ` Markus Trippelsdorf
2015-02-06 13:30 ` H.J. Lu
0 siblings, 1 reply; 17+ messages in thread
From: Markus Trippelsdorf @ 2015-02-06 13:19 UTC (permalink / raw)
To: H.J. Lu; +Cc: Richard Biener, GCC Patches
On 2015.02.06 at 05:10 -0800, H.J. Lu wrote:
> On Fri, Feb 6, 2015 at 4:56 AM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > On 2015.02.05 at 12:57 -0800, H.J. Lu wrote:
> >>
> >> We should pass handle, not file, to release_input_file.
> >> I checked it in as an obvious fix.
> >
> > This commit causes:
> >
> > % echo "int main () {}" | gcc -fuse-ld=gold -flto -x c++ -
> > ld.gold: internal error in remove_writer, at token.h:132
> > collect2: error: ld returned 1 exit status
> >
>
> ld.bfd closes fd without release_input_file being called
> and ld.gold leaks file descriptors if release_input_file
> isn't called. We can either fix gold:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=17896
>
> or revert the PR lto/64837 fix.
Given that LTO would be completely broken with any gold version in
gcc-5, a revert seems to be the only viable option.
--
Markus
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-06 13:19 ` Markus Trippelsdorf
@ 2015-02-06 13:30 ` H.J. Lu
2015-02-06 13:56 ` H.J. Lu
0 siblings, 1 reply; 17+ messages in thread
From: H.J. Lu @ 2015-02-06 13:30 UTC (permalink / raw)
To: Markus Trippelsdorf; +Cc: Richard Biener, GCC Patches
On Fri, Feb 6, 2015 at 5:18 AM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2015.02.06 at 05:10 -0800, H.J. Lu wrote:
>> On Fri, Feb 6, 2015 at 4:56 AM, Markus Trippelsdorf
>> <markus@trippelsdorf.de> wrote:
>> > On 2015.02.05 at 12:57 -0800, H.J. Lu wrote:
>> >>
>> >> We should pass handle, not file, to release_input_file.
>> >> I checked it in as an obvious fix.
>> >
>> > This commit causes:
>> >
>> > % echo "int main () {}" | gcc -fuse-ld=gold -flto -x c++ -
>> > ld.gold: internal error in remove_writer, at token.h:132
>> > collect2: error: ld returned 1 exit status
>> >
>>
>> ld.bfd closes fd without release_input_file being called
>> and ld.gold leaks file descriptors if release_input_file
>> isn't called. We can either fix gold:
>>
>> https://sourceware.org/bugzilla/show_bug.cgi?id=17896
>>
>> or revert the PR lto/64837 fix.
>
> Given that LTO would be completely broken with any gold version in
> gcc-5, a revert seems to be the only viable option.
>
Fine with me and reopen PR lto/64837.
--
H.J.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file
2015-02-06 13:30 ` H.J. Lu
@ 2015-02-06 13:56 ` H.J. Lu
0 siblings, 0 replies; 17+ messages in thread
From: H.J. Lu @ 2015-02-06 13:56 UTC (permalink / raw)
To: Markus Trippelsdorf; +Cc: Richard Biener, GCC Patches
On Fri, Feb 6, 2015 at 5:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Feb 6, 2015 at 5:18 AM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
>> On 2015.02.06 at 05:10 -0800, H.J. Lu wrote:
>>> On Fri, Feb 6, 2015 at 4:56 AM, Markus Trippelsdorf
>>> <markus@trippelsdorf.de> wrote:
>>> > On 2015.02.05 at 12:57 -0800, H.J. Lu wrote:
>>> >>
>>> >> We should pass handle, not file, to release_input_file.
>>> >> I checked it in as an obvious fix.
>>> >
>>> > This commit causes:
>>> >
>>> > % echo "int main () {}" | gcc -fuse-ld=gold -flto -x c++ -
>>> > ld.gold: internal error in remove_writer, at token.h:132
>>> > collect2: error: ld returned 1 exit status
>>> >
>>>
>>> ld.bfd closes fd without release_input_file being called
>>> and ld.gold leaks file descriptors if release_input_file
>>> isn't called. We can either fix gold:
>>>
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=17896
>>>
>>> or revert the PR lto/64837 fix.
>>
>> Given that LTO would be completely broken with any gold version in
>> gcc-5, a revert seems to be the only viable option.
>>
>
> Fine with me and reopen PR lto/64837.
>
Done with r220477.
--
H.J.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-02-06 13:56 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-28 18:51 PR lto/64837: lto plugin doesn't call ld_plugin_release_input_file H.J. Lu
2015-01-28 19:57 ` Richard Biener
2015-01-28 20:05 ` H.J. Lu
2015-01-28 20:17 ` Cary Coutant
2015-01-28 20:18 ` H.J. Lu
2015-01-29 5:07 ` H.J. Lu
2015-02-04 4:49 ` Cary Coutant
2015-02-04 8:57 ` Richard Biener
2015-02-04 15:35 ` Cary Coutant
2015-02-04 15:52 ` H.J. Lu
2015-02-05 16:43 ` H.J. Lu
2015-02-05 20:58 ` H.J. Lu
2015-02-06 12:56 ` Markus Trippelsdorf
2015-02-06 13:11 ` H.J. Lu
2015-02-06 13:19 ` Markus Trippelsdorf
2015-02-06 13:30 ` H.J. Lu
2015-02-06 13:56 ` H.J. Lu
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).