PATCH: Report plugin symbol status

PATCH: Report plugin symbol status

[H.J. Lu]

Hi,

This patch reports plugin symbol status for --verbose.  It can
be used to debug LTO plugin symbol issues.  OK for trunk?

Thanks.


H.J.
---
diff --git a/ld/ChangeLog.lto-mixed b/ld/ChangeLog.lto-mixed
index cbc0733..01c1791 100644
--- a/ld/ChangeLog.lto-mixed
+++ b/ld/ChangeLog.lto-mixed
@@ -1,3 +1,14 @@
+2011-03-02  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* lexsup.c (parse_args): Set report_plugin_symbols to TRUE
+	for --verbose.
+
+	* plugin.c (report_plugin_symbols): New.
+	(get_symbols): Report plugin symbols if report_plugin_symbols
+	is TRUE.
+
+	* plugin.h (report_plugin_symbols): New.
+
 2011-02-01  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR ld/12439
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 4baef3c..176b39e 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -1336,6 +1336,7 @@ parse_args (unsigned argc, char **argv)
 	  ldversion (1);
 	  version_printed = TRUE;
 	  trace_file_tries = TRUE;
+	  report_plugin_symbols = TRUE;
 	  overflow_cutoff_limit = -2;
 	  break;
 	case 'v':
diff --git a/ld/plugin.c b/ld/plugin.c
index 40acbdb..ab9c339 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -36,6 +36,9 @@
 #include <Windows.h>
 #endif
 
+/* Trace plugin symbols.  */
+bfd_boolean report_plugin_symbols;
+
 /* Stores a single argument passed to a plugin.  */
 typedef struct plugin_arg
 {
@@ -460,7 +463,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
       if (!blhe)
 	{
 	  syms[n].resolution = LDPR_UNKNOWN;
-	  continue;
+	  goto report_symbol;
 	}
 
       /* Determine resolution from blhe type and symbol's original type.  */
@@ -468,7 +471,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
 	  || blhe->type == bfd_link_hash_undefweak)
 	{
 	  syms[n].resolution = LDPR_UNDEF;
-	  continue;
+	  goto report_symbol;
 	}
       if (blhe->type != bfd_link_hash_defined
 	  && blhe->type != bfd_link_hash_defweak
@@ -513,7 +516,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
 	    syms[n].resolution =  LDPR_RESOLVED_DYN;
 	  else
 	    syms[n].resolution = LDPR_RESOLVED_EXEC;
-	  continue;
+	  goto report_symbol;
 	}
 
       /* Was originally def, or weakdef.  Does it prevail?  If the
@@ -526,13 +529,18 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
 	  syms[n].resolution = (ironly
 				? LDPR_PREVAILING_DEF_IRONLY
 				: LDPR_PREVAILING_DEF);
-	  continue;
+	  goto report_symbol;
 	}
 
       /* Was originally def, weakdef, or common, but has been pre-empted.  */
       syms[n].resolution = (is_ir_dummy_bfd (owner_sec->owner)
 			    ? LDPR_PREEMPTED_IR
 			    : LDPR_PREEMPTED_REG);
+
+report_symbol:
+      if (report_plugin_symbols)
+	einfo ("%P: %B: symbol `%s' definition: %d, resolution: %d\n",
+	       abfd, syms[n].name, syms[n].def, syms[n].resolution);
     }
   return LDPS_OK;
 }
diff --git a/ld/plugin.h b/ld/plugin.h
index 5b340be..5f11cf2 100644
--- a/ld/plugin.h
+++ b/ld/plugin.h
@@ -21,6 +21,8 @@
 #ifndef GLD_PLUGIN_H
 #define GLD_PLUGIN_H
 
+/* Trace plugin symbols.  */
+extern bfd_boolean report_plugin_symbols;
 
 /* This is the only forward declaration we need to avoid having
    to include the plugin-api.h header in order to use this file.  */

Re: PATCH: Report plugin symbol status

[Dave Korn]

On 02/03/2011 19:55, H.J. Lu wrote:
> Hi,
> 
> This patch reports plugin symbol status for --verbose.  It can
> be used to debug LTO plugin symbol issues.  OK for trunk?

  What sort of size inflation in the logfiles of a real-world link is this
likely to cause?  I would expect it to be a significant inflation, in which
case maybe a separate option would be more convenient than changing the
existing --verbose behaviour.

    cheers,
      DaveK

Re: PATCH: Report plugin symbol status

[H.J. Lu]

On Wed, Mar 2, 2011 at 4:45 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
> On 02/03/2011 19:55, H.J. Lu wrote:
>> Hi,
>>
>> This patch reports plugin symbol status for --verbose.  It can
>> be used to debug LTO plugin symbol issues.  OK for trunk?
>
>  What sort of size inflation in the logfiles of a real-world link is this
> likely to cause?  I would expect it to be a significant inflation, in which
> case maybe a separate option would be more convenient than changing the
> existing --verbose behaviour.
>

I can add a new switch.



-- 
H.J.

Re: PATCH: Report plugin symbol status

[Dave Korn]

On 03/03/2011 03:39, H.J. Lu wrote:
> On Wed, Mar 2, 2011 at 4:45 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
>> On 02/03/2011 19:55, H.J. Lu wrote:
>>> Hi,
>>>
>>> This patch reports plugin symbol status for --verbose.  It can
>>> be used to debug LTO plugin symbol issues.  OK for trunk?
>>  What sort of size inflation in the logfiles of a real-world link is this
>> likely to cause?  I would expect it to be a significant inflation, in which
>> case maybe a separate option would be more convenient than changing the
>> existing --verbose behaviour.
>>
> 
> I can add a new switch.

  Thank you HJ, I think it would be good.  At the moment it's very common to
use --verbose to debug problems with -L options in makefiles, and I think that
could become a good deal more difficult for end-users if there is also a huge
amount of one-entry-per-symbol-in-the-whole-project debug info coming.

    cheers,
      DaveK

Re: PATCH: Report plugin symbol status

[Alan Modra]

On Wed, Mar 02, 2011 at 07:39:34PM -0800, H.J. Lu wrote:
> On Wed, Mar 2, 2011 at 4:45 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
> > On 02/03/2011 19:55, H.J. Lu wrote:
> >> Hi,
> >>
> >> This patch reports plugin symbol status for --verbose.  It can
> >> be used to debug LTO plugin symbol issues.  OK for trunk?
> >
> >  What sort of size inflation in the logfiles of a real-world link is this
> > likely to cause?  I would expect it to be a significant inflation, in which
> > case maybe a separate option would be more convenient than changing the
> > existing --verbose behaviour.
> >
> 
> I can add a new switch.

Or give --verbose an optional arg, so you get the sym dump with
--verbose=2.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: Report plugin symbol status

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

On Thu, Mar 3, 2011 at 7:51 PM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Mar 02, 2011 at 07:39:34PM -0800, H.J. Lu wrote:
>> On Wed, Mar 2, 2011 at 4:45 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
>> > On 02/03/2011 19:55, H.J. Lu wrote:
>> >> Hi,
>> >>
>> >> This patch reports plugin symbol status for --verbose.  It can
>> >> be used to debug LTO plugin symbol issues.  OK for trunk?
>> >
>> >  What sort of size inflation in the logfiles of a real-world link is this
>> > likely to cause?  I would expect it to be a significant inflation, in which
>> > case maybe a separate option would be more convenient than changing the
>> > existing --verbose behaviour.
>> >
>>
>> I can add a new switch.
>
> Or give --verbose an optional arg, so you get the sym dump with
> --verbose=2.
>

Is this OK for trunk?

Thanks.


-- 
H.J
---
.2011-03-04  H.J. Lu  <hongjiu.lu@intel.com>

	* ld.texinfo: Document --verbose[=NUMBER].

	* lexsup.c (ld_options): Update --verbose.
	(parse_args): Set report_plugin_symbols.

	* plugin.c (report_plugin_symbols): New.
	(get_symbols): Report plugin symbols if report_plugin_symbols
	is TRUE.

	* plugin.h (report_plugin_symbols): New.

---

[-- Attachment #2: ld-plugin-verbose-1.pactch --]
[-- Type: application/octet-stream, Size: 4317 bytes --]

diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 9957d03..ed79923 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -1851,13 +1851,14 @@ Normally the linker will generate an error message for each reported
 unresolved symbol but the option @option{--warn-unresolved-symbols}
 can change this to a warning.
 
-@kindex --verbose
-@cindex verbose
+@kindex --verbose[=@var{NUMBER}]
+@cindex verbose[=@var{NUMBER}]
 @item --dll-verbose
-@itemx --verbose
+@itemx --verbose[=@var{NUMBER}]
 Display the version number for @command{ld} and list the linker emulations
 supported.  Display which input files can and cannot be opened.  Display
-the linker script being used by the linker.
+the linker script being used by the linker. If the optional @var{NUMBER}
+argument > 1, plugin symbol status will also be displayed.
 
 @kindex --version-script=@var{version-scriptfile}
 @cindex version script, symbol versions
diff --git a/ld/lexsup.c b/ld/lexsup.c
index 4baef3c..8050098 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -560,8 +560,9 @@ static const struct ld_option ld_options[] =
 		   "                                ignore-all, report-all, ignore-in-object-files,\n"
 		   "                                ignore-in-shared-libs"),
     TWO_DASHES },
-  { {"verbose", no_argument, NULL, OPTION_VERBOSE},
-    '\0', NULL, N_("Output lots of information during link"), TWO_DASHES },
+  { {"verbose", optional_argument, NULL, OPTION_VERBOSE},
+    '\0', N_("[=NUMBER]"),
+    N_("Output lots of information during link"), TWO_DASHES },
   { {"dll-verbose", no_argument, NULL, OPTION_VERBOSE}, /* Linux.  */
     '\0', NULL, NULL, NO_HELP },
   { {"version-script", required_argument, NULL, OPTION_VERSION_SCRIPT },
@@ -1337,6 +1338,13 @@ parse_args (unsigned argc, char **argv)
 	  version_printed = TRUE;
 	  trace_file_tries = TRUE;
 	  overflow_cutoff_limit = -2;
+	  if (optarg != NULL)
+	    {
+	      char *end;
+	      report_plugin_symbols = strtoul (optarg, &end, 0);
+	      if (*end)
+		einfo (_("%P%F: invalid number `%s'\n"), optarg);
+	    }
 	  break;
 	case 'v':
 	  ldversion (0);
diff --git a/ld/plugin.c b/ld/plugin.c
index 40acbdb..2d915ff 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -36,6 +36,9 @@
 #include <Windows.h>
 #endif
 
+/* Report plugin symbols.  */
+int report_plugin_symbols;
+
 /* Stores a single argument passed to a plugin.  */
 typedef struct plugin_arg
 {
@@ -460,7 +463,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
       if (!blhe)
 	{
 	  syms[n].resolution = LDPR_UNKNOWN;
-	  continue;
+	  goto report_symbol;
 	}
 
       /* Determine resolution from blhe type and symbol's original type.  */
@@ -468,7 +471,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
 	  || blhe->type == bfd_link_hash_undefweak)
 	{
 	  syms[n].resolution = LDPR_UNDEF;
-	  continue;
+	  goto report_symbol;
 	}
       if (blhe->type != bfd_link_hash_defined
 	  && blhe->type != bfd_link_hash_defweak
@@ -513,7 +516,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
 	    syms[n].resolution =  LDPR_RESOLVED_DYN;
 	  else
 	    syms[n].resolution = LDPR_RESOLVED_EXEC;
-	  continue;
+	  goto report_symbol;
 	}
 
       /* Was originally def, or weakdef.  Does it prevail?  If the
@@ -526,13 +529,18 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms)
 	  syms[n].resolution = (ironly
 				? LDPR_PREVAILING_DEF_IRONLY
 				: LDPR_PREVAILING_DEF);
-	  continue;
+	  goto report_symbol;
 	}
 
       /* Was originally def, weakdef, or common, but has been pre-empted.  */
       syms[n].resolution = (is_ir_dummy_bfd (owner_sec->owner)
 			    ? LDPR_PREEMPTED_IR
 			    : LDPR_PREEMPTED_REG);
+
+report_symbol:
+      if (report_plugin_symbols > 1)
+	einfo ("%P: %B: symbol `%s' definition: %d, resolution: %d\n",
+	       abfd, syms[n].name, syms[n].def, syms[n].resolution);
     }
   return LDPS_OK;
 }
diff --git a/ld/plugin.h b/ld/plugin.h
index 5b340be..ebb8773 100644
--- a/ld/plugin.h
+++ b/ld/plugin.h
@@ -21,6 +21,8 @@
 #ifndef GLD_PLUGIN_H
 #define GLD_PLUGIN_H
 
+/* Report plugin symbols.  */
+extern int report_plugin_symbols;
 
 /* This is the only forward declaration we need to avoid having
    to include the plugin-api.h header in order to use this file.  */

Re: PATCH: Report plugin symbol status

[Alan Modra]

On Fri, Mar 04, 2011 at 12:46:11PM -0800, H.J. Lu wrote:
> 	* ld.texinfo: Document --verbose[=NUMBER].
> 
> 	* lexsup.c (ld_options): Update --verbose.
> 	(parse_args): Set report_plugin_symbols.
> 
> 	* plugin.c (report_plugin_symbols): New.
> 	(get_symbols): Report plugin symbols if report_plugin_symbols
> 	is TRUE.
> 
> 	* plugin.h (report_plugin_symbols): New.

OK, except

@@ -1337,6 +1338,13 @@ parse_args (unsigned argc, char **argv)
 	  version_printed = TRUE;
 	  trace_file_tries = TRUE;
 	  overflow_cutoff_limit = -2;
+	  if (optarg != NULL)
+	    {
+	      char *end;
+	      report_plugin_symbols = strtoul (optarg, &end, 0);
+	      if (*end)
+		einfo (_("%P%F: invalid number `%s'\n"), optarg);
+	    }
 	  break;
 	case 'v':
 	  ldversion (0);

I think it would be cleaner to make report_plugin_symbols a boolean
here, rather than leaving it as the --verbose arg and testing for
greater than one later.  Alternatively, rename the variable
"verbose_arg" or somesuch.  Either way is fine.

+report_symbol:
+      if (report_plugin_symbols > 1)
+	einfo ("%P: %B: symbol `%s' definition: %d, resolution: %d\n",
+	       abfd, syms[n].name, syms[n].def, syms[n].resolution);

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: Report plugin symbol status

[H.J. Lu]

On Fri, Mar 4, 2011 at 2:26 PM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Mar 04, 2011 at 12:46:11PM -0800, H.J. Lu wrote:
>>       * ld.texinfo: Document --verbose[=NUMBER].
>>
>>       * lexsup.c (ld_options): Update --verbose.
>>       (parse_args): Set report_plugin_symbols.
>>
>>       * plugin.c (report_plugin_symbols): New.
>>       (get_symbols): Report plugin symbols if report_plugin_symbols
>>       is TRUE.
>>
>>       * plugin.h (report_plugin_symbols): New.
>
> OK, except
>
> @@ -1337,6 +1338,13 @@ parse_args (unsigned argc, char **argv)
>          version_printed = TRUE;
>          trace_file_tries = TRUE;
>          overflow_cutoff_limit = -2;
> +         if (optarg != NULL)
> +           {
> +             char *end;
> +             report_plugin_symbols = strtoul (optarg, &end, 0);
> +             if (*end)
> +               einfo (_("%P%F: invalid number `%s'\n"), optarg);
> +           }
>          break;
>        case 'v':
>          ldversion (0);
>
> I think it would be cleaner to make report_plugin_symbols a boolean
> here, rather than leaving it as the --verbose arg and testing for
> greater than one later.  Alternatively, rename the variable
> "verbose_arg" or somesuch.  Either way is fine.


This is what I checked in.

Thanks.


-- 
H.J.
---
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 4f5f2c8..bdf4b01 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,16 @@
+2011-03-04  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* ld.texinfo: Document --verbose[=NUMBER].
+
+	* lexsup.c (ld_options): Update --verbose.
+	(parse_args): Set report_plugin_symbols.
+
+	* plugin.c (report_plugin_symbols): New.
+	(get_symbols): Report plugin symbols if report_plugin_symbols
+	is TRUE.
+
+	* plugin.h (report_plugin_symbols): New.
+
 2011-03-01  H.J. Lu  <hongjiu.lu@intel.com>

 	PR ld/12529
diff --git a/ld/ld.texinfo b/ld/ld.texinfo
index 9957d03..ed79923 100644
--- a/ld/ld.texinfo
+++ b/ld/ld.texinfo
@@ -1851,13 +1851,14 @@ Normally the linker will generate an error
message for each reported
 unresolved symbol but the option @option{--warn-unresolved-symbols}
 can change this to a warning.

-@kindex --verbose
-@cindex verbose
+@kindex --verbose[=@var{NUMBER}]
+@cindex verbose[=@var{NUMBER}]
 @item --dll-verbose
-@itemx --verbose
+@itemx --verbose[=@var{NUMBER}]
 Display the version number for @command{ld} and list the linker emulations
 supported.  Display which input files can and cannot be opened.  Display
-the linker script being used by the linker.
+the linker script being used by the linker. If the optional @var{NUMBER}
+argument > 1, plugin symbol status will also be displayed.

 @kindex --version-script=@var{version-scriptfile}
 @cindex version script, symbol versions
diff --git a/ld/lexsup.c b/ld/lexsup.c
index acb63fb..8cf96eb 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -560,8 +560,9 @@ static const struct ld_option ld_options[] =
 		   "                                ignore-all, report-all,
ignore-in-object-files,\n"
 		   "                                ignore-in-shared-libs"),
     TWO_DASHES },
-  { {"verbose", no_argument, NULL, OPTION_VERBOSE},
-    '\0', NULL, N_("Output lots of information during link"), TWO_DASHES },
+  { {"verbose", optional_argument, NULL, OPTION_VERBOSE},
+    '\0', N_("[=NUMBER]"),
+    N_("Output lots of information during link"), TWO_DASHES },
   { {"dll-verbose", no_argument, NULL, OPTION_VERBOSE}, /* Linux.  */
     '\0', NULL, NULL, NO_HELP },
   { {"version-script", required_argument, NULL, OPTION_VERSION_SCRIPT },
@@ -1326,6 +1327,14 @@ parse_args (unsigned argc, char **argv)
 	  version_printed = TRUE;
 	  trace_file_tries = TRUE;
 	  overflow_cutoff_limit = -2;
+	  if (optarg != NULL)
+	    {
+	      char *end;
+	      int level = strtoul (optarg, &end, 0);
+	      if (*end)
+		einfo (_("%P%F: invalid number `%s'\n"), optarg);
+	      report_plugin_symbols = level > 1;
+	    }
 	  break;
 	case 'v':
 	  ldversion (0);
diff --git a/ld/plugin.c b/ld/plugin.c
index 7ac2cb8..a68aea3 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -36,6 +36,9 @@
 #include <Windows.h>
 #endif

+/* Report plugin symbols.  */
+bfd_boolean report_plugin_symbols;
+
 /* The suffix to append to the name of the real (claimed) object file
    when generating a dummy BFD to hold the IR symbols sent from the
    plugin.  */
@@ -463,7 +466,7 @@ get_symbols (const void *handle, int nsyms, struct
ld_plugin_symbol *syms)
       if (!blhe)
 	{
 	  syms[n].resolution = LDPR_UNKNOWN;
-	  continue;
+	  goto report_symbol;
 	}

       /* Determine resolution from blhe type and symbol's original type.  */
@@ -471,7 +474,7 @@ get_symbols (const void *handle, int nsyms, struct
ld_plugin_symbol *syms)
 	  || blhe->type == bfd_link_hash_undefweak)
 	{
 	  syms[n].resolution = LDPR_UNDEF;
-	  continue;
+	  goto report_symbol;
 	}
       if (blhe->type != bfd_link_hash_defined
 	  && blhe->type != bfd_link_hash_defweak
@@ -516,7 +519,7 @@ get_symbols (const void *handle, int nsyms, struct
ld_plugin_symbol *syms)
 	    syms[n].resolution =  LDPR_RESOLVED_DYN;
 	  else
 	    syms[n].resolution = LDPR_RESOLVED_EXEC;
-	  continue;
+	  goto report_symbol;
 	}

       /* Was originally def, or weakdef.  Does it prevail?  If the
@@ -529,13 +532,18 @@ get_symbols (const void *handle, int nsyms,
struct ld_plugin_symbol *syms)
 	  syms[n].resolution = (ironly
 				? LDPR_PREVAILING_DEF_IRONLY
 				: LDPR_PREVAILING_DEF);
-	  continue;
+	  goto report_symbol;
 	}

       /* Was originally def, weakdef, or common, but has been pre-empted.  */
-      syms[n].resolution = is_ir_dummy_bfd (owner_sec->owner)
-	? LDPR_PREEMPTED_IR
-	: LDPR_PREEMPTED_REG;
+      syms[n].resolution = (is_ir_dummy_bfd (owner_sec->owner)
+			    ? LDPR_PREEMPTED_IR
+			    : LDPR_PREEMPTED_REG);
+
+report_symbol:
+      if (report_plugin_symbols)
+	einfo ("%P: %B: symbol `%s' definition: %d, resolution: %d\n",
+	       abfd, syms[n].name, syms[n].def, syms[n].resolution);
     }
   return LDPS_OK;
 }
diff --git a/ld/plugin.h b/ld/plugin.h
index 5b340be..6ba8fa8 100644
--- a/ld/plugin.h
+++ b/ld/plugin.h
@@ -21,6 +21,8 @@
 #ifndef GLD_PLUGIN_H
 #define GLD_PLUGIN_H

+/* Report plugin symbols.  */
+extern bfd_boolean report_plugin_symbols;

 /* This is the only forward declaration we need to avoid having
    to include the plugin-api.h header in order to use this file.  *