plugin: Call dlclose before return in try_load_plugin

Message ID 20200218125016.140100-1-hjl.tools@gmail.com
State New
Headers show
Series
  • plugin: Call dlclose before return in try_load_plugin
Related show

Commit Message

H.J. Lu Feb. 18, 2020, 12:50 p.m.
Since plugin can be used only once in try_load_plugin, call dlclose
before return.

OK for master and 2.34 branch?

	PR binutils/25355
	* plugin.c (plugin_list_entry): Remove handle.
	(try_load_plugin): Call dlclose before return.
---
 bfd/plugin.c | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

-- 
2.24.1

Comments

Alan Modra Feb. 19, 2020, 4:59 a.m. | #1
On Tue, Feb 18, 2020 at 04:50:16AM -0800, H.J. Lu wrote:
> Since plugin can be used only once in try_load_plugin, call dlclose

> before return.

> 

> OK for master and 2.34 branch?

> 

> 	PR binutils/25355

> 	* plugin.c (plugin_list_entry): Remove handle.

> 	(try_load_plugin): Call dlclose before return.


Looks OK to me.

-- 
Alan Modra
Australia Development Lab, IBM
H.J. Lu Feb. 19, 2020, 11:35 a.m. | #2
On Tue, Feb 18, 2020 at 8:59 PM Alan Modra <amodra@gmail.com> wrote:
>

> On Tue, Feb 18, 2020 at 04:50:16AM -0800, H.J. Lu wrote:

> > Since plugin can be used only once in try_load_plugin, call dlclose

> > before return.

> >

> > OK for master and 2.34 branch?

> >

> >       PR binutils/25355

> >       * plugin.c (plugin_list_entry): Remove handle.

> >       (try_load_plugin): Call dlclose before return.

>

> Looks OK to me.

>


This is the patch I am checking in.  It kept

  if (current_plugin)
    memset (current_plugin, 0,
                   offsetof (struct plugin_list_entry, next));

-- 
H.J.
From d9d92bacb7adb4bab0c26eb7458203345a0faf13 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 18 Feb 2020 04:22:34 -0800
Subject: [PATCH] plugin: Call dlclose before return in try_load_plugin

Since plugin can be used only once in try_load_plugin, call dlclose
before return.

	PR binutils/25355
	* plugin.c (plugin_list_entry): Remove handle.
	(try_load_plugin): Call dlclose before return.
---
 bfd/plugin.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/bfd/plugin.c b/bfd/plugin.c
index 47c3439042c..93d562b9fed 100644
--- a/bfd/plugin.c
+++ b/bfd/plugin.c
@@ -126,7 +126,6 @@ message (int level ATTRIBUTE_UNUSED,
 struct plugin_list_entry
 {
   /* These must be initialized for each IR object with LTO wrapper.  */
-  void *handle;
   ld_plugin_claim_file_handler claim_file;
   ld_plugin_all_symbols_read_handler all_symbols_read;
   ld_plugin_all_symbols_read_handler cleanup_handler;
@@ -596,22 +595,18 @@ try_load_plugin (const char *pname,
 		 struct plugin_list_entry *plugin_list_iter,
 		 bfd *abfd, bfd_boolean build_list_p)
 {
-  void *plugin_handle = NULL;
+  void *plugin_handle;
   struct ld_plugin_tv tv[12];
   int i;
   ld_plugin_onload onload;
   enum ld_plugin_status status;
+  int result = 0;
 
   /* NB: Each object is independent.  Reuse the previous plugin from
      the last run will lead to wrong result.  */
   if (current_plugin)
-    {
-      if (current_plugin->handle)
-	dlclose (current_plugin->handle);
-      memset (current_plugin, 0,
-	      offsetof (struct plugin_list_entry, next));
-      current_plugin = NULL;
-    }
+    memset (current_plugin, 0,
+	    offsetof (struct plugin_list_entry, next));
 
   if (plugin_list_iter)
     pname = plugin_list_iter->plugin_name;
@@ -628,12 +623,12 @@ try_load_plugin (const char *pname,
       size_t length_plugin_name = strlen (pname) + 1;
       char *plugin_name = bfd_malloc (length_plugin_name);
       if (plugin_name == NULL)
-	return 0;
+	goto short_circuit;
       plugin_list_iter = bfd_malloc (sizeof *plugin_list_iter);
       if (plugin_list_iter == NULL)
 	{
 	  free (plugin_name);
-	  return 0;
+	  goto short_circuit;
 	}
       /* Make a copy of PNAME since PNAME from load_plugin () will be
 	 freed.  */
@@ -644,13 +639,13 @@ try_load_plugin (const char *pname,
       plugin_list = plugin_list_iter;
     }
 
-  plugin_list_iter->handle = plugin_handle;
+  current_plugin = plugin_list_iter;
   if (build_list_p)
-    return 0;
+    goto short_circuit;
 
   onload = dlsym (plugin_handle, "onload");
   if (!onload)
-    return 0;
+    goto short_circuit;
 
   i = 0;
   tv[i].tv_tag = LDPT_MESSAGE;
@@ -703,28 +698,30 @@ try_load_plugin (const char *pname,
   tv[i].tv_tag = LDPT_NULL;
   tv[i].tv_u.tv_val = 0;
 
-  current_plugin = plugin_list_iter;
-
   /* LTO plugin will call handler hooks to set up plugin handlers.  */
   status = (*onload)(tv);
 
   if (status != LDPS_OK)
-    return 0;
+    goto short_circuit;
 
   if (current_plugin->lto_wrapper
       && setup_lto_wrapper_env (current_plugin))
-    return 0;
+    goto short_circuit;
 
   abfd->plugin_format = bfd_plugin_no;
 
   if (!current_plugin->claim_file)
-    return 0;
+    goto short_circuit;
 
   if (!try_claim (abfd))
-    return 0;
+    goto short_circuit;
 
   abfd->plugin_format = bfd_plugin_yes;
-  return 1;
+  result = 1;
+
+short_circuit:
+  dlclose (plugin_handle);
+  return result;
 }
 
 /* There may be plugin libraries in lib/bfd-plugins.  */
H.J. Lu Feb. 19, 2020, 1:45 p.m. | #3
On Wed, Feb 19, 2020 at 3:35 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>

> On Tue, Feb 18, 2020 at 8:59 PM Alan Modra <amodra@gmail.com> wrote:

> >

> > On Tue, Feb 18, 2020 at 04:50:16AM -0800, H.J. Lu wrote:

> > > Since plugin can be used only once in try_load_plugin, call dlclose

> > > before return.

> > >

> > > OK for master and 2.34 branch?

> > >

> > >       PR binutils/25355

> > >       * plugin.c (plugin_list_entry): Remove handle.

> > >       (try_load_plugin): Call dlclose before return.

> >

> > Looks OK to me.

> >

>

> This is the patch I am checking in.  It kept

>

>   if (current_plugin)

>     memset (current_plugin, 0,

>                    offsetof (struct plugin_list_entry, next));

>


Hi Nick,

Is it OK for backport to 2.34 branch?

-- 
H.J.
Nick Clifton Feb. 19, 2020, 4:49 p.m. | #4
Hi H.J.

>>>>       PR binutils/25355

>>>>       * plugin.c (plugin_list_entry): Remove handle.

>>>>       (try_load_plugin): Call dlclose before return.


> Is it OK for backport to 2.34 branch?


Yes - please do.

Cheers
  Nick

Patch

diff --git a/bfd/plugin.c b/bfd/plugin.c
index 47c3439042c..84b96a20733 100644
--- a/bfd/plugin.c
+++ b/bfd/plugin.c
@@ -126,7 +126,6 @@  message (int level ATTRIBUTE_UNUSED,
 struct plugin_list_entry
 {
   /* These must be initialized for each IR object with LTO wrapper.  */
-  void *handle;
   ld_plugin_claim_file_handler claim_file;
   ld_plugin_all_symbols_read_handler all_symbols_read;
   ld_plugin_all_symbols_read_handler cleanup_handler;
@@ -596,22 +595,12 @@  try_load_plugin (const char *pname,
 		 struct plugin_list_entry *plugin_list_iter,
 		 bfd *abfd, bfd_boolean build_list_p)
 {
-  void *plugin_handle = NULL;
+  void *plugin_handle;
   struct ld_plugin_tv tv[12];
   int i;
   ld_plugin_onload onload;
   enum ld_plugin_status status;
-
-  /* NB: Each object is independent.  Reuse the previous plugin from
-     the last run will lead to wrong result.  */
-  if (current_plugin)
-    {
-      if (current_plugin->handle)
-	dlclose (current_plugin->handle);
-      memset (current_plugin, 0,
-	      offsetof (struct plugin_list_entry, next));
-      current_plugin = NULL;
-    }
+  int result = 0;
 
   if (plugin_list_iter)
     pname = plugin_list_iter->plugin_name;
@@ -628,12 +617,12 @@  try_load_plugin (const char *pname,
       size_t length_plugin_name = strlen (pname) + 1;
       char *plugin_name = bfd_malloc (length_plugin_name);
       if (plugin_name == NULL)
-	return 0;
+	goto short_circuit;
       plugin_list_iter = bfd_malloc (sizeof *plugin_list_iter);
       if (plugin_list_iter == NULL)
 	{
 	  free (plugin_name);
-	  return 0;
+	  goto short_circuit;
 	}
       /* Make a copy of PNAME since PNAME from load_plugin () will be
 	 freed.  */
@@ -644,13 +633,13 @@  try_load_plugin (const char *pname,
       plugin_list = plugin_list_iter;
     }
 
-  plugin_list_iter->handle = plugin_handle;
+  current_plugin = plugin_list_iter;
   if (build_list_p)
-    return 0;
+    goto short_circuit;
 
   onload = dlsym (plugin_handle, "onload");
   if (!onload)
-    return 0;
+    goto short_circuit;
 
   i = 0;
   tv[i].tv_tag = LDPT_MESSAGE;
@@ -703,28 +692,32 @@  try_load_plugin (const char *pname,
   tv[i].tv_tag = LDPT_NULL;
   tv[i].tv_u.tv_val = 0;
 
-  current_plugin = plugin_list_iter;
-
   /* LTO plugin will call handler hooks to set up plugin handlers.  */
   status = (*onload)(tv);
 
   if (status != LDPS_OK)
-    return 0;
+    goto short_circuit;
 
   if (current_plugin->lto_wrapper
       && setup_lto_wrapper_env (current_plugin))
-    return 0;
+    goto short_circuit;
 
   abfd->plugin_format = bfd_plugin_no;
 
   if (!current_plugin->claim_file)
-    return 0;
+    goto short_circuit;
 
   if (!try_claim (abfd))
-    return 0;
+    goto short_circuit;
 
   abfd->plugin_format = bfd_plugin_yes;
-  return 1;
+  result = 1;
+
+short_circuit:
+  /* NB: Each object is independent.  Reuse the previous plugin from
+     the last run will lead to wrong result.  */
+  dlclose (plugin_handle);
+  return result;
 }
 
 /* There may be plugin libraries in lib/bfd-plugins.  */