| Message ID | 20150128181242.GA4350@intel.com |
|---|---|
| State | New |
| Headers | show |
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;
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; > >
>>>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
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.
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;