Re: [PATCH] Provide string description of definition, visibility and resolution in LTO plug-in.

[Martin Liška <mliska at suse dot cz>]

On 3/13/19 1:28 PM, Nick Clifton wrote:
> Hi Martin,
>  
>   Sorry for the delay in reviewing.

Hi.

That's fine.

> 
>> I would like to enhance get_symbols function to report string
>> representation of definition, visibility and resolution.
>> It's easier for users to read it.
> 
> Agreed, but ...
> 
> @@ -777,9 +808,11 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms,
>        syms[n].resolution = res;
>        if (report_plugin_symbols)
>  	einfo (_("%P: %pB: symbol `%s' "
> -		 "definition: %d, visibility: %d, resolution: %d\n"),
> +		 "definition: %s, visibility: %s, resolution: %s\n"),
>  	       abfd, syms[n].name,
> -	       syms[n].def, syms[n].visibility, res);
> +	       lto_kind_str[syms[n].def],
> +	       lto_visibility_str[syms[n].visibility],
> +	       lto_resolution_str[res]);
> 
> You need to be more paranoid.  If one of the values is not in the
> expected range, then you will have an illegal or unexpected memory
> dereference via the array accesses.
> 
> My suggestion - create a set of small new functions which return
> the name of a particular value, and which handle unknown values.
> eg:
> 
>   const char *
>   get_lto_kind (unsigned int index)
>   {
>      const char *lto_kind_str[5] =
>      {
>       "DEF",
>       "WEAKDEF",
>       "UNDEF",
>       "WEAKUNDEF",
>       "COMMON"
>      };
> 
>     if (index < ARRAY_SIZE (lto_kind_str))
>       return lto_kind_str [index];
> 
>     const char buf [1024];
>     sprintf (buf, _("unknown definition value %x"), index);
>     return buf;
>   }
> 
> Then use these new functions in the einfo() statement.
> 
> Cheers
>   Nick
> 

Good point, I reworked that.

Patch survives tests on x86_64-linux-gnu.

Martin

>From e6fda49aa3d7ad69f8034fd10bfa876e15fd3b8b Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 26 Feb 2019 15:19:12 +0100
Subject: [PATCH] Provide string description of definition, visibility and
 resolution in LTO plug-in.

ld/ChangeLog:

2019-02-26  Martin Liska  <mliska@suse.cz>

	* plugin.c (get_symbols): Add lto_kind_str, lto_resolution_str,
	lto_visibility_str and use then to inform about plugin-symbols.
	* testsuite/ld-plugin/plugin-12.d: Adjust expected pattern.
---
 ld/plugin.c                        | 73 +++++++++++++++++++++++++++++-
 ld/testsuite/ld-plugin/plugin-12.d |  8 ++--
 2 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/ld/plugin.c b/ld/plugin.c
index ea1a7f7064..fd3dcbc8d3 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -659,6 +659,73 @@ is_visible_from_outside (struct ld_plugin_symbol *lsym,
   return FALSE;
 }
 
+/* Return LTO kind string name that corresponds to INDEX enum value.  */
+static const char *
+get_lto_kind (unsigned int index)
+{
+  const char *lto_kind_str[5] =
+  {
+    "DEF",
+    "WEAKDEF",
+    "UNDEF",
+    "WEAKUNDEF",
+    "COMMON"
+  };
+
+  if (index < ARRAY_SIZE (lto_kind_str))
+    return lto_kind_str [index];
+
+  char *buf = xmalloc (64);
+  sprintf (buf, _("unknown LTO kind value %x"), index);
+  return buf;
+}
+
+/* Return LTO resolution string name that corresponds to INDEX enum value.  */
+static const char *
+get_lto_resolution (unsigned int index)
+{
+  static const char *lto_resolution_str[10] =
+  {
+    "UNKNOWN",
+    "UNDEF",
+    "PREVAILING_DEF",
+    "PREVAILING_DEF_IRONLY",
+    "PREEMPTED_REG",
+    "PREEMPTED_IR",
+    "RESOLVED_IR",
+    "RESOLVED_EXEC",
+    "RESOLVED_DYN",
+    "PREVAILING_DEF_IRONLY_EXP",
+  };
+
+  if (index < ARRAY_SIZE (lto_resolution_str))
+    return lto_resolution_str [index];
+
+  char *buf = xmalloc (64);
+  sprintf (buf, _("unknown LTO resolution value %x"), index);
+  return buf;
+}
+
+/* Return LTO visibility string name that corresponds to INDEX enum value.  */
+static const char *
+get_lto_visibility (unsigned int index)
+{
+  const char *lto_visibility_str[4] =
+  {
+    "DEFAULT",
+    "PROTECTED",
+    "INTERNAL",
+    "HIDDEN"
+  };
+
+  if (index < ARRAY_SIZE (lto_visibility_str))
+    return lto_visibility_str [index];
+
+  char *buf = xmalloc (64);
+  sprintf (buf, _("unknown LTO visibility value %x"), index);
+  return buf;
+}
+
 /* Get the symbol resolution info for a plugin-claimed input file.  */
 static enum ld_plugin_status
 get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms,
@@ -777,9 +844,11 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms,
       syms[n].resolution = res;
       if (report_plugin_symbols)
 	einfo (_("%P: %pB: symbol `%s' "
-		 "definition: %d, visibility: %d, resolution: %d\n"),
+		 "definition: %s, visibility: %s, resolution: %s\n"),
 	       abfd, syms[n].name,
-	       syms[n].def, syms[n].visibility, res);
+	       get_lto_kind (syms[n].def),
+	       get_lto_visibility (syms[n].visibility),
+	       get_lto_resolution (res));
     }
   return LDPS_OK;
 }
diff --git a/ld/testsuite/ld-plugin/plugin-12.d b/ld/testsuite/ld-plugin/plugin-12.d
index 10d772553d..35eae1e00a 100644
--- a/ld/testsuite/ld-plugin/plugin-12.d
+++ b/ld/testsuite/ld-plugin/plugin-12.d
@@ -1,6 +1,6 @@
 #...
-.*: symbol `func' definition: 0, visibility: 0, resolution: 2
-.*: symbol `func1' definition: 0, visibility: 1, resolution: 3
-.*: symbol `func2' definition: 0, visibility: 2, resolution: 3
-.*: symbol `func3' definition: 0, visibility: 3, resolution: 3
+.*: symbol `func' definition: DEF, visibility: DEFAULT, resolution: PREVAILING_DEF
+.*: symbol `func1' definition: DEF, visibility: PROTECTED, resolution: PREVAILING_DEF_IRONLY
+.*: symbol `func2' definition: DEF, visibility: INTERNAL, resolution: PREVAILING_DEF_IRONLY
+.*: symbol `func3' definition: DEF, visibility: HIDDEN, resolution: PREVAILING_DEF_IRONLY
 #pass
-- 
2.21.0