Re: get_symbol_v2 linker plugin interface

[Alan Modra <amodra at gmail dot com>]

On Thu, Oct 06, 2011 at 08:56:01PM +1030, Alan Modra wrote:
> ld/
> 	PR ld/13229
> 	* plugin.c (tv_header_tags): Add LDPT_GET_SYMBOLS_V2.
> 	(set_tv_header): Handle it.  Adjust LDPT_GET_SYMBOLS.  Return void.
> 	(get_symbols): Add def_ironly_exp param.  Return that value for
> 	syms exported from shared libs.
> 	(get_symbols_v1, get_symbols_v2): New wrapper functions.
> 	* testplug.c: Update for above changes.

The above patch changed the call site for is_visible_from_outside()
such that the is_ir_dummy_bfd() check is now redundant.  I also
noticed today that the plugin code was testing link_info.shared when
the proper test for a shared lib is !link_info.executable.  "Easy
fixed" says I, then found that link_info.executable wasn't even set
at the point where the first plugin call is made.  The plugin init
does need to happen early (to have plugin_notice installed before any
symbols are created), but not that early!  So this patch moves some
code around as well as fixing the plugin bug.

	* ldmain.c (main): Move code twiddling various config and link_info
	bits to..
	* lexsup.c (parse_args): ..here.  Move plugin_load_plugins call
	into main.
	* plugin.c (set_tv_header): Test link_info.executable, not
	link_info.shared.
	(is_visible_from_outside): Likewise.  Delete redundant
	is_ir_dummy_bfd check and "section" parameter.

Index: ld/ldmain.c
===================================================================
RCS file: /cvs/src/src/ld/ldmain.c,v
retrieving revision 1.160
diff -u -p -r1.160 ldmain.c
--- ld/ldmain.c	15 Sep 2011 16:03:03 -0000	1.160
+++ ld/ldmain.c	10 Oct 2011 12:04:10 -0000
@@ -297,73 +297,13 @@ main (int argc, char **argv)
   if (config.hash_table_size != 0)
     bfd_hash_set_default_size (config.hash_table_size);
 
-  ldemul_set_symbols ();
-
-  if (link_info.relocatable)
-    {
-      if (command_line.check_section_addresses < 0)
-	command_line.check_section_addresses = 0;
-      if (link_info.shared)
-	einfo (_("%P%F: -r and -shared may not be used together\n"));
-    }
-
-  /* We may have -Bsymbolic, -Bsymbolic-functions, --dynamic-list-data,
-     --dynamic-list-cpp-new, --dynamic-list-cpp-typeinfo and
-     --dynamic-list FILE.  -Bsymbolic and -Bsymbolic-functions are
-     for shared libraries.  -Bsymbolic overrides all others and vice
-     versa.  */
-  switch (command_line.symbolic)
-    {
-    case symbolic_unset:
-      break;
-    case symbolic:
-      /* -Bsymbolic is for shared library only.  */
-      if (link_info.shared)
-	{
-	  link_info.symbolic = TRUE;
-	  /* Should we free the unused memory?  */
-	  link_info.dynamic_list = NULL;
-	  command_line.dynamic_list = dynamic_list_unset;
-	}
-      break;
-    case symbolic_functions:
-      /* -Bsymbolic-functions is for shared library only.  */
-      if (link_info.shared)
-	command_line.dynamic_list = dynamic_list_data;
-      break;
-    }
-
-  switch (command_line.dynamic_list)
-    {
-    case dynamic_list_unset:
-      break;
-    case dynamic_list_data:
-      link_info.dynamic_data = TRUE;
-    case dynamic_list:
-      link_info.dynamic = TRUE;
-      break;
-    }
-
-  if (! link_info.shared)
-    {
-      if (command_line.filter_shlib)
-	einfo (_("%P%F: -F may not be used without -shared\n"));
-      if (command_line.auxiliary_filters)
-	einfo (_("%P%F: -f may not be used without -shared\n"));
-    }
-
-  if (! link_info.shared || link_info.pie)
-    link_info.executable = TRUE;
+#ifdef ENABLE_PLUGINS
+  /* Now all the plugin arguments have been gathered, we can load them.  */
+  if (plugin_load_plugins ())
+    einfo (_("%P%F: %s: error loading plugin\n"), plugin_error_plugin ());
+#endif /* ENABLE_PLUGINS */
 
-  /* Treat ld -r -s as ld -r -S -x (i.e., strip all local symbols).  I
-     don't see how else this can be handled, since in this case we
-     must preserve all externally visible symbols.  */
-  if (link_info.relocatable && link_info.strip == strip_all)
-    {
-      link_info.strip = strip_debugger;
-      if (link_info.discard == discard_sec_merge)
-	link_info.discard = discard_all;
-    }
+  ldemul_set_symbols ();
 
   /* If we have not already opened and parsed a linker script,
      try the default script from command line first.  */
Index: ld/lexsup.c
===================================================================
RCS file: /cvs/src/src/ld/lexsup.c,v
retrieving revision 1.127
diff -u -p -r1.127 lexsup.c
--- ld/lexsup.c	16 Jul 2011 07:58:40 -0000	1.127
+++ ld/lexsup.c	10 Oct 2011 12:04:11 -0000
@@ -1564,11 +1564,71 @@ parse_args (unsigned argc, char **argv)
     /* FIXME: Should we allow emulations a chance to set this ?  */
     link_info.unresolved_syms_in_shared_libs = how_to_report_unresolved_symbols;
 
-#ifdef ENABLE_PLUGINS
-  /* Now all the plugin arguments have been gathered, we can load them.  */
-  if (plugin_load_plugins ())
-    einfo (_("%P%F: %s: error loading plugin\n"), plugin_error_plugin ());
-#endif /* ENABLE_PLUGINS */
+  if (link_info.relocatable)
+    {
+      if (command_line.check_section_addresses < 0)
+	command_line.check_section_addresses = 0;
+      if (link_info.shared)
+	einfo (_("%P%F: -r and -shared may not be used together\n"));
+    }
+
+  /* We may have -Bsymbolic, -Bsymbolic-functions, --dynamic-list-data,
+     --dynamic-list-cpp-new, --dynamic-list-cpp-typeinfo and
+     --dynamic-list FILE.  -Bsymbolic and -Bsymbolic-functions are
+     for shared libraries.  -Bsymbolic overrides all others and vice
+     versa.  */
+  switch (command_line.symbolic)
+    {
+    case symbolic_unset:
+      break;
+    case symbolic:
+      /* -Bsymbolic is for shared library only.  */
+      if (link_info.shared)
+	{
+	  link_info.symbolic = TRUE;
+	  /* Should we free the unused memory?  */
+	  link_info.dynamic_list = NULL;
+	  command_line.dynamic_list = dynamic_list_unset;
+	}
+      break;
+    case symbolic_functions:
+      /* -Bsymbolic-functions is for shared library only.  */
+      if (link_info.shared)
+	command_line.dynamic_list = dynamic_list_data;
+      break;
+    }
+
+  switch (command_line.dynamic_list)
+    {
+    case dynamic_list_unset:
+      break;
+    case dynamic_list_data:
+      link_info.dynamic_data = TRUE;
+    case dynamic_list:
+      link_info.dynamic = TRUE;
+      break;
+    }
+
+  if (! link_info.shared)
+    {
+      if (command_line.filter_shlib)
+	einfo (_("%P%F: -F may not be used without -shared\n"));
+      if (command_line.auxiliary_filters)
+	einfo (_("%P%F: -f may not be used without -shared\n"));
+    }
+
+  if (! link_info.shared || link_info.pie)
+    link_info.executable = TRUE;
+
+  /* Treat ld -r -s as ld -r -S -x (i.e., strip all local symbols).  I
+     don't see how else this can be handled, since in this case we
+     must preserve all externally visible symbols.  */
+  if (link_info.relocatable && link_info.strip == strip_all)
+    {
+      link_info.strip = strip_debugger;
+      if (link_info.discard == discard_sec_merge)
+	link_info.discard = discard_all;
+    }
 }
 
 /* Add the (colon-separated) elements of DIRLIST_PTR to the
Index: ld/plugin.c
===================================================================
RCS file: /cvs/src/src/ld/plugin.c,v
retrieving revision 1.41
diff -u -p -r1.41 plugin.c
--- ld/plugin.c	6 Oct 2011 10:07:17 -0000	1.41
+++ ld/plugin.c	10 Oct 2011 12:04:12 -0000
@@ -442,18 +442,14 @@ release_input_file (const void *handle)
 /* Return TRUE if a defined symbol might be reachable from outside the
    universe of claimed objects.  */
 static inline bfd_boolean
-is_visible_from_outside (struct ld_plugin_symbol *lsym, asection *section,
+is_visible_from_outside (struct ld_plugin_symbol *lsym,
 			 struct bfd_link_hash_entry *blhe)
 {
   struct bfd_sym_chain *sym;
 
-  /* Section's owner may be NULL if it is the absolute
-     section, fortunately is_ir_dummy_bfd handles that.  */
-  if (!is_ir_dummy_bfd (section->owner))
-    return TRUE;
   if (link_info.relocatable)
     return TRUE;
-  if (link_info.export_dynamic || link_info.shared)
+  if (link_info.export_dynamic || !link_info.executable)
     {
       /* Check if symbol is hidden by version script.  */
       if (bfd_hide_sym_by_version (link_info.version_info,
@@ -580,7 +576,7 @@ get_symbols (const void *handle, int nsy
 	     symbol is externally visible.  */
 	  if (blhe->non_ir_ref)
 	    res = LDPR_PREVAILING_DEF;
-	  else if (is_visible_from_outside (&syms[n], owner_sec, blhe))
+	  else if (is_visible_from_outside (&syms[n], blhe))
 	    res = def_ironly_exp;
 	}
 
@@ -701,7 +697,7 @@ set_tv_header (struct ld_plugin_tv *tv)
 	case LDPT_LINKER_OUTPUT:
 	  TVU(val) = (link_info.relocatable
 		      ? LDPO_REL
-		      : (link_info.shared ? LDPO_DYN : LDPO_EXEC));
+		      : link_info.executable ? LDPO_EXEC : LDPO_DYN);
 	  break;
 	case LDPT_OUTPUT_NAME:
 	  TVU(string) = output_filename;

-- 
Alan Modra
Australia Development Lab, IBM