[PATCH] Plugin: Treat each object as independent

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Feb 11, 2020 at 4:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 6:31 PM Alan Modra <amodra@gmail.com> wrote:
> >
> > On Mon, Feb 10, 2020 at 05:22:24PM -0800, H.J. Lu wrote:
> > > +  if (lto_symbol_found)
> > > +    {
> > > +      current_plugin->real_nsyms = real_nsyms;
> > > +      current_plugin->real_syms = real_syms;
> > > +      /* NB: We can't close RBFD which own the real symbol info.  */
> > > +      current_plugin->real_bfd = rbfd;
> > > +    }
> > > +  else
> > > +    bfd_close (rbfd);
> >
> > I think you might want to free real_syms here on else branch.  OK with
> > that fix.
>
> I am checking in this patch to avoid uninitialized memory.

This patch is needed to support multiple plugins in
${libdir}/bfd-plugins directory.
OK for master branch?

Thanks.

-- 
H.J.

From f592aa9ac97f18dc978026856289a6703189bf66 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 11 Feb 2020 07:48:45 -0800
Subject: [PATCH] Plugin: Treat each object as independent

Since plugin treats each object as independent, we must do a fresh dlopen
of plugin for each object.

	PR binutils/25355
	* plugin.c (try_claim): Always clean up for LTO wrapper.
	(try_load_plugin): Treat each object as independent.  Create a
	copy for plugin name.
---
 bfd/plugin.c | 76 ++++++++++++++++++++++++++--------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/bfd/plugin.c b/bfd/plugin.c
index 5681a6a291..d941677154 100644
--- a/bfd/plugin.c
+++ b/bfd/plugin.c
@@ -549,9 +549,8 @@ try_claim (bfd *abfd)
   struct ld_plugin_input_file file;
 
   file.handle = abfd;
-  if (!bfd_plugin_open_input (abfd, &file))
-    return 0;
-  if (current_plugin->claim_file)
+  if (bfd_plugin_open_input (abfd, &file)
+      && current_plugin->claim_file)
     {
       current_plugin->claim_file (&file, &claimed);
       if (claimed)
@@ -577,14 +576,18 @@ try_claim (bfd *abfd)
 	    }
 	}
 
-      if (current_plugin->lto_wrapper)
-	{
-	  /* Clean up for LTO wrapper.  */
-	  unlink (current_plugin->resolution_file);
-	  free (current_plugin->resolution_option);
-	}
+      close (file.fd);
     }
-  close (file.fd);
+
+  if (current_plugin->lto_wrapper)
+    {
+      /* Clean up for LTO wrapper.  NB: Resolution file and option
+	 have been created regardless if an IR object is claimed or
+	 not.  */
+      unlink (current_plugin->resolution_file);
+      free (current_plugin->resolution_option);
+    }
+
   return claimed;
 }
 
@@ -600,16 +603,15 @@ try_load_plugin (const char *pname, bfd *abfd, int *has_plugin_p)
 
   *has_plugin_p = 0;
 
-  /* NB: Each object is inddependent.  Reuse the previous plugin from
-     the last LTO wrapper run will lead to wrong LTO data.  */
-  if (current_plugin
-      && current_plugin->handle
-      && current_plugin->lto_wrapper
-      && strcmp (current_plugin->plugin_name, pname) == 0)
+  /* NB: Each object is independent.  Reuse the previous plugin from
+     the last run will lead to wrong result.  */
+  if (current_plugin)
     {
-      dlclose (current_plugin->handle);
+      if (current_plugin->handle)
+	dlclose (current_plugin->handle);
       memset (current_plugin, 0,
 	      offsetof (struct plugin_list_entry, next));
+      current_plugin = NULL;
     }
 
   plugin_handle = dlopen (pname, RTLD_NOW);
@@ -622,31 +624,30 @@ try_load_plugin (const char *pname, bfd *abfd, int *has_plugin_p)
   for (plugin_list_iter = plugin_list;
        plugin_list_iter;
        plugin_list_iter = plugin_list_iter->next)
+    if (strcmp (plugin_list_iter->plugin_name, pname) == 0)
+      break;
+
+  if (plugin_list_iter == NULL)
     {
-      if (plugin_handle == plugin_list_iter->handle)
+      size_t length_plugin_name = strlen (pname) + 1;
+      char *plugin_name = bfd_malloc (length_plugin_name);
+      if (plugin_name == NULL)
+	return 0;
+      plugin_list_iter = bfd_malloc (sizeof *plugin_list_iter);
+      if (plugin_list_iter == NULL)
 	{
-	  dlclose (plugin_handle);
-	  if (!plugin_list_iter->claim_file)
-	    return 0;
-
-	  register_claim_file (plugin_list_iter->claim_file);
-	  current_plugin = plugin_list_iter;
-	  goto have_claim_file;
+	  free (plugin_name);
+	  return 0;
 	}
-      else if (plugin_list_iter->lto_wrapper
-	       && strcmp (plugin_list_iter->plugin_name, pname) == 0)
-	goto have_lto_wrapper;
+      /* Make a copy of PNAME since PNAME from load_plugin () will be
+	 freed.  */
+      memcpy (plugin_name, pname, length_plugin_name);
+      memset (plugin_list_iter, 0, sizeof (*plugin_list_iter));
+      plugin_list_iter->plugin_name = plugin_name;
+      plugin_list_iter->next = plugin_list;
+      plugin_list = plugin_list_iter;
     }
 
-  plugin_list_iter = bfd_malloc (sizeof *plugin_list_iter);
-  if (plugin_list_iter == NULL)
-    return 0;
-  memset (plugin_list_iter, 0, sizeof (*plugin_list_iter));
-  plugin_list_iter->plugin_name = pname;
-  plugin_list_iter->next = plugin_list;
-  plugin_list = plugin_list_iter;
-
-have_lto_wrapper:
   plugin_list_iter->handle = plugin_handle;
 
   onload = dlsym (plugin_handle, "onload");
@@ -716,7 +717,6 @@ have_lto_wrapper:
       && setup_lto_wrapper_env (current_plugin))
     return 0;
 
-have_claim_file:
   *has_plugin_p = 1;
 
   abfd->plugin_format = bfd_plugin_no;
-- 
2.24.1