Re: New gold plugin interface for getting --wrap symbols

Re: New gold plugin interface for getting --wrap symbols

[Sriraman Tallam via binutils]

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

re-sending as plain text.


* plugin.cc (get_wrap_symbols): New plugin interface.
(load): Add get_wrap_symbols to transfer vector.
* plugin-api.h (ld_plugin_get_wrap_symbols): New plugin interface.
* testsuite/plugin_test.c (onload): Call and check get_wrap_symbols
interface.
* testsuite/plugin_test_wrap_symbols.sh: New test script.
* testsuite/plugin_test_wrap_symbols_1.cc: New file.
* testsuite/plugin_test_wrap_symbols_2.cc: New file.
* testsuite/Makefile.am (plugin_test_wrap_symbols): New test.
* testsuite/Makefile.in: Regenerate.


On Tue, Nov 21, 2017 at 11:30 AM, Sriraman Tallam <tmsriram@google.com> wrote:
> Hi,
>
>      LLVM ThinLTO needs a gold plugin API to get the list of symbols that
> are wrapped using the --wrap option to avoid garbage collection of some of
> the associated functions like __wrap_symbol and __real_symbol.
>
>      This patch adds an interface, get_wrap_symbols, to get the count and
> the list of symbols that are wrapped.
>
>
> * plugin.cc (get_wrap_symbols): New plugin interface.
> (load): Add get_wrap_symbols to transfer vector.
> * plugin-api.h (ld_plugin_get_wrap_symbols): New plugin interface.
> * testsuite/plugin_test.c (onload): Call and check get_wrap_symbols
> interface.
> * testsuite/plugin_test_wrap_symbols.sh: New test script.
> * testsuite/plugin_test_wrap_symbols_1.cc: New file.
> * testsuite/plugin_test_wrap_symbols_2.cc: New file.
> * testsuite/Makefile.am (plugin_test_wrap_symbols): New test.
> * testsuite/Makefile.in: Regenerate.
>
>
> Thanks
> Sri

[-- Attachment #2: plugin_wrap_symbol_patch.txt --]
[-- Type: text/plain, Size: 10575 bytes --]

diff --git a/gold/plugin.cc b/gold/plugin.cc
index 5ea23b5bdd..b80cd12c2f 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -167,6 +167,9 @@ static enum ld_plugin_status
 get_input_section_size(const struct ld_plugin_section section,
                        uint64_t* secsize);
 
+static enum ld_plugin_status
+get_wrap_symbols(uint64_t *num_symbols, const char ***wrap_symbol_list);
+
 };
 
 #endif // ENABLE_PLUGINS
@@ -211,7 +214,7 @@ Plugin::load()
   sscanf(ver, "%d.%d", &major, &minor);
 
   // Allocate and populate a transfer vector.
-  const int tv_fixed_size = 29;
+  const int tv_fixed_size = 30;
 
   int tv_size = this->args_.size() + tv_fixed_size;
   ld_plugin_tv* tv = new ld_plugin_tv[tv_size];
@@ -345,6 +348,10 @@ Plugin::load()
   tv[i].tv_tag = LDPT_GET_INPUT_SECTION_SIZE;
   tv[i].tv_u.tv_get_input_section_size = get_input_section_size;
 
+  ++i;
+  tv[i].tv_tag = LDPT_GET_WRAP_SYMBOLS;
+  tv[i].tv_u.tv_get_wrap_symbols = get_wrap_symbols;
+
   ++i;
   tv[i].tv_tag = LDPT_NULL;
   tv[i].tv_u.tv_val = 0;
@@ -1797,6 +1804,29 @@ get_input_section_size(const struct ld_plugin_section section,
   return LDPS_OK;
 }
 
+static enum ld_plugin_status
+get_wrap_symbols(uint64_t *count, const char ***wrap_symbols)
+{
+  gold_assert(parameters->options().has_plugins());
+  std::vector<const char *> wrapped_symbols;
+  uint64_t total = 0;
+
+  for (options::String_set::const_iterator
+       it = parameters->options().wrap_begin();
+       it != parameters->options().wrap_end(); ++it) {
+    total++;
+    wrapped_symbols.push_back(it->c_str());
+  }
+
+  *count = total;
+  if (total == 0)
+    return LDPS_OK;
+
+  *wrap_symbols = new const char *[total];
+  std::copy(wrapped_symbols.begin(), wrapped_symbols.end(), *wrap_symbols);
+  return LDPS_OK;
+}
+
 
 // Specify the ordering of sections in the final layout. The sections are
 // specified as (handle,shndx) pairs in the two arrays in the order in
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index d8426db62f..802cf4446f 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2318,6 +2318,19 @@ plugin_test_12: export_dynamic_plugin.o gcctestdir/ld plugin_test.so export_dyna
 plugin_test_12.err: plugin_test_12
 	@touch plugin_test_12.err
 
+check_PROGRAMS += plugin_test_wrap_symbols
+check_SCRIPTS += plugin_test_wrap_symbols.sh
+check_DATA += plugin_test_wrap_symbols.err
+MOSTLYCLEANFILES += plugin_test_wrap_symbols.err
+plugin_test_wrap_symbols_1.o: plugin_test_wrap_symbols_1.cc
+	$(COMPILE) -c -o $@ $<
+plugin_test_wrap_symbols_2.o: plugin_test_wrap_symbols_2.cc
+	$(COMPILE) -c -o $@ $<
+plugin_test_wrap_symbols: plugin_test_wrap_symbols_1.o plugin_test_wrap_symbols_2.o gcctestdir/ld plugin_test.so
+	$(CXXLINK) -Bgcctestdir/ -Wl,--plugin,"./plugin_test.so" -Wl,--wrap=hello,--wrap=jello plugin_test_wrap_symbols_1.o plugin_test_wrap_symbols_2.o 2>plugin_test_wrap_symbols.err
+plugin_test_wrap_symbols.err: plugin_test_wrap_symbols
+	@touch plugin_test_wrap_symbols.err
+
 check_PROGRAMS += plugin_test_start_lib
 check_SCRIPTS += plugin_test_start_lib.sh
 check_DATA += plugin_test_start_lib.err
diff --git a/gold/testsuite/plugin_test.c b/gold/testsuite/plugin_test.c
index e8c8a5dfcf..fbd32977ef 100644
--- a/gold/testsuite/plugin_test.c
+++ b/gold/testsuite/plugin_test.c
@@ -68,6 +68,7 @@ static ld_plugin_get_input_section_name get_input_section_name = NULL;
 static ld_plugin_get_input_section_contents get_input_section_contents = NULL;
 static ld_plugin_update_section_order update_section_order = NULL;
 static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
+static ld_plugin_get_wrap_symbols get_wrap_symbols = NULL;
 
 #define MAXOPTS 10
 
@@ -158,6 +159,9 @@ onload(struct ld_plugin_tv *tv)
 	case LDPT_ALLOW_SECTION_ORDERING:
 	  allow_section_ordering = *entry->tv_u.tv_allow_section_ordering;
 	  break;
+	case LDPT_GET_WRAP_SYMBOLS:
+	  get_wrap_symbols = *entry->tv_u.tv_get_wrap_symbols;
+	  break;
         default:
           break;
         }
@@ -247,6 +251,28 @@ onload(struct ld_plugin_tv *tv)
       return LDPS_ERR;
     }
 
+  if (get_wrap_symbols == NULL)
+    {
+      fprintf(stderr, "tv_get_wrap_symbols interface missing\n");
+      return LDPS_ERR;
+    }
+  else
+    {
+      const char **wrap_symbols;
+      uint64_t count = 0;
+      if (get_wrap_symbols(&count, &wrap_symbols) == LDPS_OK)
+	{
+	  (*message)(LDPL_INFO, "Number of wrap symbols = %lu", count);
+ 	  for (; count > 0; --count)
+            (*message)(LDPL_INFO, "Wrap symbol %s", wrap_symbols[count - 1]);
+	}
+      else
+	{
+          fprintf(stderr, "tv_get_wrap_symbols interface call failed\n");
+          return LDPS_ERR;
+	}
+    }
+
   return LDPS_OK;
 }
 
diff --git a/gold/testsuite/plugin_test_wrap_symbols.sh b/gold/testsuite/plugin_test_wrap_symbols.sh
index e69de29bb2..2133bf0a82 100755
--- a/gold/testsuite/plugin_test_wrap_symbols.sh
+++ b/gold/testsuite/plugin_test_wrap_symbols.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+# plugin_test_wrap_symbols.sh -- a test case for the plugin API.
+
+# Copyright (C) 2017 onwards Free Software Foundation, Inc.
+# Written by Teresa Johnson <tejohnson@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# This file goes with plugin_test.c, a simple plug-in library that
+# exercises the basic interfaces and prints out version numbers and
+# options passed to the plugin.
+
+# This checks if the get_wrap_symbols plugin interface works as
+# expected.
+
+check()
+{
+    if ! grep -q "$2" "$1"
+    then
+	echo "Did not find expected output in $1:"
+	echo "   $2"
+	echo ""
+	echo "Actual output below:"
+	cat "$1"
+	exit 1
+    fi
+}
+
+check plugin_test_wrap_symbols.err "API version:"
+check plugin_test_wrap_symbols.err "gold version:"
+check plugin_test_wrap_symbols.err "Number of wrap symbols = 2"
+check plugin_test_wrap_symbols.err "Wrap symbol hello"
+check plugin_test_wrap_symbols.err "Wrap symbol jello"
+check plugin_test_wrap_symbols.err "cleanup hook called"
+
+exit 0
diff --git a/gold/testsuite/plugin_test_wrap_symbols_1.cc b/gold/testsuite/plugin_test_wrap_symbols_1.cc
index e69de29bb2..90587efbb9 100644
--- a/gold/testsuite/plugin_test_wrap_symbols_1.cc
+++ b/gold/testsuite/plugin_test_wrap_symbols_1.cc
@@ -0,0 +1,40 @@
+/* plugin_test_wrap_symbols -- a test case for gold
+
+   Copyright (C) 2017 onwards Free Software Foundation, Inc.
+   Written by Sriraman Tallam <tmsriram@google.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+extern "C" {
+int __real_hello();
+int __wrap_hello()
+{
+  return __real_hello();
+}
+int hello();
+int __real_jello();
+int __wrap_jello()
+{
+  return __real_jello();
+}
+int jello();
+int main()
+{
+  return 0;
+}
+}
diff --git a/gold/testsuite/plugin_test_wrap_symbols_2.cc b/gold/testsuite/plugin_test_wrap_symbols_2.cc
index e69de29bb2..902cc3c076 100644
--- a/gold/testsuite/plugin_test_wrap_symbols_2.cc
+++ b/gold/testsuite/plugin_test_wrap_symbols_2.cc
@@ -0,0 +1,33 @@
+/* plugin_test_wrap_symbols -- a test case for gold
+
+   Copyright (C) 2017 onwards Free Software Foundation, Inc.
+   Written by Sriraman Tallam <tmsriram@google.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+extern "C" {
+int hello()
+{
+  return 0;
+}
+
+int jello()
+{
+  return 0;
+}
+}
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 3a3e8b456d..901369ad4d 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -365,6 +365,11 @@ enum ld_plugin_status
 (*ld_plugin_get_input_section_size) (const struct ld_plugin_section section,
                                      uint64_t *secsize);
 
+typedef
+enum ld_plugin_status
+(*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
+                               const char ***wrap_symbol_list);
+
 enum ld_plugin_level
 {
   LDPL_INFO,
@@ -407,7 +412,8 @@ enum ld_plugin_tag
   LDPT_UNIQUE_SEGMENT_FOR_SECTIONS = 27,
   LDPT_GET_SYMBOLS_V3 = 28,
   LDPT_GET_INPUT_SECTION_ALIGNMENT = 29,
-  LDPT_GET_INPUT_SECTION_SIZE = 30
+  LDPT_GET_INPUT_SECTION_SIZE = 30,
+  LDPT_GET_WRAP_SYMBOLS = 31
 };
 
 /* The plugin transfer vector.  */
@@ -441,6 +447,7 @@ struct ld_plugin_tv
     ld_plugin_unique_segment_for_sections tv_unique_segment_for_sections;
     ld_plugin_get_input_section_alignment tv_get_input_section_alignment;
     ld_plugin_get_input_section_size tv_get_input_section_size;
+    ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
   } tv_u;
 };
 

Re: New gold plugin interface for getting --wrap symbols

[Cary Coutant]

>>      LLVM ThinLTO needs a gold plugin API to get the list of symbols that
>> are wrapped using the --wrap option to avoid garbage collection of some of
>> the associated functions like __wrap_symbol and __real_symbol.
>>
>>      This patch adds an interface, get_wrap_symbols, to get the count and
>> the list of symbols that are wrapped.

There's a difference between the list of --wrap options passed to the
linker (which is what this API returns), and a list of which symbols
actually got wrapped. How is this useful compared to simply
pattern-matching for symbols whose name begins with "__wrap" and
"__real" (which will give you the symbols that were actually wrapped)?

Assuming this is what you want...

+  std::vector<const char *> wrapped_symbols;
+  uint64_t total = 0;
+
+  for (options::String_set::const_iterator
+       it = parameters->options().wrap_begin();
+       it != parameters->options().wrap_end(); ++it) {
+    total++;
+    wrapped_symbols.push_back(it->c_str());
+  }

total is redundant -- just use wrapped_symbols.size().

Furthermore, if you add an extra member function to DEFINE_set that
exposes the String_set::size() method, e.g.:

  options::String_set::size_type
  varname__##_size() const
  { return this->varname__##_.value.size(); }

... you could avoid copying to and from the intermediate vector.

(I *think* all of the various alternative implementations we might use
for Unordered_set provide a constant-time size() method.)

-cary

Re: New gold plugin interface for getting --wrap symbols

[Teresa Johnson via binutils]

On Mon, Nov 27, 2017 at 10:31 PM, Cary Coutant <ccoutant@gmail.com> wrote:

> >>      LLVM ThinLTO needs a gold plugin API to get the list of symbols
> that
> >> are wrapped using the --wrap option to avoid garbage collection of some
> of
> >> the associated functions like __wrap_symbol and __real_symbol.
> >>
> >>      This patch adds an interface, get_wrap_symbols, to get the count
> and
> >> the list of symbols that are wrapped.
>
> There's a difference between the list of --wrap options passed to the
> linker (which is what this API returns), and a list of which symbols
> actually got wrapped.


Can you clarify what cases would be different?


> How is this useful compared to simply
> pattern-matching for symbols whose name begins with "__wrap" and
> "__real" (which will give you the symbols that were actually wrapped)?
>

My concern is that this is a little hacky and would result in the plugin
not accurately reflecting the linker behavior. For ThinLTO we need to build
an accurate whole program graph, and want to reflect the wrapping that
occurs in the linker. If we pattern match but the user doesn't pass --wrap,
for example, that would be problematic.

As a side note, a follow-on patch is planned to get similar info to the
plugin for --defsym, for which pattern matching on symbol names won't
suffice.


>
> Assuming this is what you want...
>

I'll let Sri comment on the rest.

Thanks,
Teresa


>
> +  std::vector<const char *> wrapped_symbols;
> +  uint64_t total = 0;
> +
> +  for (options::String_set::const_iterator
> +       it = parameters->options().wrap_begin();
> +       it != parameters->options().wrap_end(); ++it) {
> +    total++;
> +    wrapped_symbols.push_back(it->c_str());
> +  }
>
> total is redundant -- just use wrapped_symbols.size().
>
> Furthermore, if you add an extra member function to DEFINE_set that
> exposes the String_set::size() method, e.g.:
>
>   options::String_set::size_type
>   varname__##_size() const
>   { return this->varname__##_.value.size(); }
>
> ... you could avoid copying to and from the intermediate vector.
>
> (I *think* all of the various alternative implementations we might use
> for Unordered_set provide a constant-time size() method.)
>
> -cary
>



-- 
Teresa Johnson |  Software Engineer |  tejohnson@google.com |  408-460-2413

Re: New gold plugin interface for getting --wrap symbols

[Cary Coutant]

>> There's a difference between the list of --wrap options passed to the
>> linker (which is what this API returns), and a list of which symbols
>> actually got wrapped.
>
> Can you clarify what cases would be different?

The --wrap options are requests to wrap a symbol *if we see it*, so
there may be symbols listed there that don't exist in the program.

>> How is this useful compared to simply
>> pattern-matching for symbols whose name begins with "__wrap" and
>> "__real" (which will give you the symbols that were actually wrapped)?
>
> My concern is that this is a little hacky and would result in the plugin not
> accurately reflecting the linker behavior. For ThinLTO we need to build an
> accurate whole program graph, and want to reflect the wrapping that occurs
> in the linker. If we pattern match but the user doesn't pass --wrap, for
> example, that would be problematic.

The __wrap_ and __real_ prefixes are in the namespace reserved for the
system's use, so it shouldn't be a problem for valid programs.

Still, I'm not objecting to the idea of querying for --wrap options --
just pointing out the differences.

I wonder if, as an alternative implementation, it might be reasonable
to have the linker simply forward certain "interesting" options from
its command line to the plugin as LDPT_OPTION entries in the transfer
vector (as if they had also been written as --plugin-opt options). Or,
since that might affect older plugins, we could make a new tag and
pass them as LDPT_LINKER_OPTION entries.

> As a side note, a follow-on patch is planned to get similar info to the
> plugin for --defsym, for which pattern matching on symbol names won't
> suffice.

Yes, that will probably need a new interface. There's no similar issue
here, though, since --defsym options always define the symbol. Since
we record the origin of symbol definitions in the symbol table, I
don't think you'd need or want to see the actual options -- you may
just want a new field in ld_plugin_symbol, or a new API to query that
attribute for each symbol.

In addition to --defsym options, I'd think you'd also care about
symbol assignments in scripts.

-cary

Re: New gold plugin interface for getting --wrap symbols

[Sriraman Tallam via binutils]

On Fri, Dec 1, 2017 at 2:21 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>>> There's a difference between the list of --wrap options passed to the
>>> linker (which is what this API returns), and a list of which symbols
>>> actually got wrapped.
>>
>> Can you clarify what cases would be different?
>
> The --wrap options are requests to wrap a symbol *if we see it*, so
> there may be symbols listed there that don't exist in the program.
>
>>> How is this useful compared to simply
>>> pattern-matching for symbols whose name begins with "__wrap" and
>>> "__real" (which will give you the symbols that were actually wrapped)?
>>
>> My concern is that this is a little hacky and would result in the plugin not
>> accurately reflecting the linker behavior. For ThinLTO we need to build an
>> accurate whole program graph, and want to reflect the wrapping that occurs
>> in the linker. If we pattern match but the user doesn't pass --wrap, for
>> example, that would be problematic.
>
> The __wrap_ and __real_ prefixes are in the namespace reserved for the
> system's use, so it shouldn't be a problem for valid programs.
>
> Still, I'm not objecting to the idea of querying for --wrap options --
> just pointing out the differences.
>
> I wonder if, as an alternative implementation, it might be reasonable
> to have the linker simply forward certain "interesting" options from
> its command line to the plugin as LDPT_OPTION entries in the transfer
> vector (as if they had also been written as --plugin-opt options). Or,
> since that might affect older plugins, we could make a new tag and
> pass them as LDPT_LINKER_OPTION entries.

This is interesting but I am afraid we may run the risk of writing
another option parser for the plugin.  I haven't thought about how to
do this.

>
>> As a side note, a follow-on patch is planned to get similar info to the
>> plugin for --defsym, for which pattern matching on symbol names won't
>> suffice.
>
> Yes, that will probably need a new interface. There's no similar issue
> here, though, since --defsym options always define the symbol. Since
> we record the origin of symbol definitions in the symbol table, I
> don't think you'd need or want to see the actual options -- you may
> just want a new field in ld_plugin_symbol, or a new API to query that
> attribute for each symbol.
>
> In addition to --defsym options, I'd think you'd also care about
> symbol assignments in scripts.
>
> -cary

Re: New gold plugin interface for getting --wrap symbols

[Sriraman Tallam via binutils]

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

On Mon, Nov 27, 2017 at 10:31 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>>>      LLVM ThinLTO needs a gold plugin API to get the list of symbols that
>>> are wrapped using the --wrap option to avoid garbage collection of some of
>>> the associated functions like __wrap_symbol and __real_symbol.
>>>
>>>      This patch adds an interface, get_wrap_symbols, to get the count and
>>> the list of symbols that are wrapped.
>
> There's a difference between the list of --wrap options passed to the
> linker (which is what this API returns), and a list of which symbols
> actually got wrapped. How is this useful compared to simply
> pattern-matching for symbols whose name begins with "__wrap" and
> "__real" (which will give you the symbols that were actually wrapped)?
>
> Assuming this is what you want...
>
> +  std::vector<const char *> wrapped_symbols;
> +  uint64_t total = 0;
> +
> +  for (options::String_set::const_iterator
> +       it = parameters->options().wrap_begin();
> +       it != parameters->options().wrap_end(); ++it) {
> +    total++;
> +    wrapped_symbols.push_back(it->c_str());
> +  }
>
> total is redundant -- just use wrapped_symbols.size().
>
> Furthermore, if you add an extra member function to DEFINE_set that
> exposes the String_set::size() method, e.g.:
>
>   options::String_set::size_type
>   varname__##_size() const
>   { return this->varname__##_.value.size(); }
>
> ... you could avoid copying to and from the intermediate vector.
>
> (I *think* all of the various alternative implementations we might use
> for Unordered_set provide a constant-time size() method.)

I have attached the patch with those changes.

 * plugin.cc (get_wrap_symbols): New plugin interface.
(load): Add get_wrap_symbols to transfer vector.
* plugin-api.h (ld_plugin_get_wrap_symbols): New plugin interface.
* options.h (options::String_set::size_type): New macro function.
* testsuite/plugin_test.c (onload): Call and check get_wrap_symbols
interface.
* testsuite/plugin_test_wrap_symbols.sh: New test script.
* testsuite/plugin_test_wrap_symbols_1.cc: New file.
* testsuite/plugin_test_wrap_symbols_2.cc: New file.
* testsuite/Makefile.am (plugin_test_wrap_symbols): New test.
* testsuite/Makefile.in: Regenerate.




Thanks
Sri

>
> -cary

[-- Attachment #2: plugin_wrap_symbol_patch.txt --]
[-- Type: text/plain, Size: 11939 bytes --]

2017-11-21  Sriraman Tallam  <tmsriram@google.com>

	* plugin.cc (get_wrap_symbols): New plugin interface.
	(load): Add get_wrap_symbols to transfer vector.
	* plugin-api.h (ld_plugin_get_wrap_symbols): New plugin interface.
	* options.h (options::String_set::size_type): New macro function.
	* testsuite/plugin_test.c (onload): Call and check get_wrap_symbols
	interface.
	* testsuite/plugin_test_wrap_symbols.sh: New test script.
	* testsuite/plugin_test_wrap_symbols_1.cc: New file.
	* testsuite/plugin_test_wrap_symbols_2.cc: New file.
	* testsuite/Makefile.am (plugin_test_wrap_symbols): New test.
	* testsuite/Makefile.in: Regenerate.

diff --git a/gold/options.h b/gold/options.h
index 2a43e8d4e0..b726f746b2 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -471,7 +471,11 @@ struct Struct_special : public Struct_var
 									  \
   options::String_set::const_iterator					  \
   varname__##_end() const						  \
-  { return this->varname__##_.value.end(); }
+  { return this->varname__##_.value.end(); }                              \
+                                                                          \
+  options::String_set::size_type                                          \
+  varname__##_size() const                                                \
+  { return this->varname__##_.value.size(); }                             \
 
 // When you have a list of possible values (expressed as string)
 // After helparg__ should come an initializer list, like
diff --git a/gold/plugin.cc b/gold/plugin.cc
index 5ea23b5bdd..f1c32ecaf5 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -167,6 +167,9 @@ static enum ld_plugin_status
 get_input_section_size(const struct ld_plugin_section section,
                        uint64_t* secsize);
 
+static enum ld_plugin_status
+get_wrap_symbols(uint64_t *num_symbols, const char ***wrap_symbol_list);
+
 };
 
 #endif // ENABLE_PLUGINS
@@ -211,7 +214,7 @@ Plugin::load()
   sscanf(ver, "%d.%d", &major, &minor);
 
   // Allocate and populate a transfer vector.
-  const int tv_fixed_size = 29;
+  const int tv_fixed_size = 30;
 
   int tv_size = this->args_.size() + tv_fixed_size;
   ld_plugin_tv* tv = new ld_plugin_tv[tv_size];
@@ -345,6 +348,10 @@ Plugin::load()
   tv[i].tv_tag = LDPT_GET_INPUT_SECTION_SIZE;
   tv[i].tv_u.tv_get_input_section_size = get_input_section_size;
 
+  ++i;
+  tv[i].tv_tag = LDPT_GET_WRAP_SYMBOLS;
+  tv[i].tv_u.tv_get_wrap_symbols = get_wrap_symbols;
+
   ++i;
   tv[i].tv_tag = LDPT_NULL;
   tv[i].tv_u.tv_val = 0;
@@ -1797,6 +1804,25 @@ get_input_section_size(const struct ld_plugin_section section,
   return LDPS_OK;
 }
 
+static enum ld_plugin_status
+get_wrap_symbols(uint64_t *count, const char ***wrap_symbols)
+{
+  gold_assert(parameters->options().has_plugins());
+  *count = parameters->options().wrap_size();
+
+  if (*count == 0)
+    return LDPS_OK; 
+
+  *wrap_symbols = new const char *[*count];
+  int i = 0;
+  for (options::String_set::const_iterator
+       it = parameters->options().wrap_begin();
+       it != parameters->options().wrap_end(); ++it, ++i) {
+    (*wrap_symbols)[i] = it->c_str();
+  }
+  return LDPS_OK;
+}
+
 
 // Specify the ordering of sections in the final layout. The sections are
 // specified as (handle,shndx) pairs in the two arrays in the order in
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index d8426db62f..802cf4446f 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2318,6 +2318,19 @@ plugin_test_12: export_dynamic_plugin.o gcctestdir/ld plugin_test.so export_dyna
 plugin_test_12.err: plugin_test_12
 	@touch plugin_test_12.err
 
+check_PROGRAMS += plugin_test_wrap_symbols
+check_SCRIPTS += plugin_test_wrap_symbols.sh
+check_DATA += plugin_test_wrap_symbols.err
+MOSTLYCLEANFILES += plugin_test_wrap_symbols.err
+plugin_test_wrap_symbols_1.o: plugin_test_wrap_symbols_1.cc
+	$(COMPILE) -c -o $@ $<
+plugin_test_wrap_symbols_2.o: plugin_test_wrap_symbols_2.cc
+	$(COMPILE) -c -o $@ $<
+plugin_test_wrap_symbols: plugin_test_wrap_symbols_1.o plugin_test_wrap_symbols_2.o gcctestdir/ld plugin_test.so
+	$(CXXLINK) -Bgcctestdir/ -Wl,--plugin,"./plugin_test.so" -Wl,--wrap=hello,--wrap=jello plugin_test_wrap_symbols_1.o plugin_test_wrap_symbols_2.o 2>plugin_test_wrap_symbols.err
+plugin_test_wrap_symbols.err: plugin_test_wrap_symbols
+	@touch plugin_test_wrap_symbols.err
+
 check_PROGRAMS += plugin_test_start_lib
 check_SCRIPTS += plugin_test_start_lib.sh
 check_DATA += plugin_test_start_lib.err
diff --git a/gold/testsuite/plugin_test.c b/gold/testsuite/plugin_test.c
index e8c8a5dfcf..fbd32977ef 100644
--- a/gold/testsuite/plugin_test.c
+++ b/gold/testsuite/plugin_test.c
@@ -68,6 +68,7 @@ static ld_plugin_get_input_section_name get_input_section_name = NULL;
 static ld_plugin_get_input_section_contents get_input_section_contents = NULL;
 static ld_plugin_update_section_order update_section_order = NULL;
 static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
+static ld_plugin_get_wrap_symbols get_wrap_symbols = NULL;
 
 #define MAXOPTS 10
 
@@ -158,6 +159,9 @@ onload(struct ld_plugin_tv *tv)
 	case LDPT_ALLOW_SECTION_ORDERING:
 	  allow_section_ordering = *entry->tv_u.tv_allow_section_ordering;
 	  break;
+	case LDPT_GET_WRAP_SYMBOLS:
+	  get_wrap_symbols = *entry->tv_u.tv_get_wrap_symbols;
+	  break;
         default:
           break;
         }
@@ -247,6 +251,28 @@ onload(struct ld_plugin_tv *tv)
       return LDPS_ERR;
     }
 
+  if (get_wrap_symbols == NULL)
+    {
+      fprintf(stderr, "tv_get_wrap_symbols interface missing\n");
+      return LDPS_ERR;
+    }
+  else
+    {
+      const char **wrap_symbols;
+      uint64_t count = 0;
+      if (get_wrap_symbols(&count, &wrap_symbols) == LDPS_OK)
+	{
+	  (*message)(LDPL_INFO, "Number of wrap symbols = %lu", count);
+ 	  for (; count > 0; --count)
+            (*message)(LDPL_INFO, "Wrap symbol %s", wrap_symbols[count - 1]);
+	}
+      else
+	{
+          fprintf(stderr, "tv_get_wrap_symbols interface call failed\n");
+          return LDPS_ERR;
+	}
+    }
+
   return LDPS_OK;
 }
 
diff --git a/gold/testsuite/plugin_test_wrap_symbols.sh b/gold/testsuite/plugin_test_wrap_symbols.sh
index e69de29bb2..2133bf0a82 100755
--- a/gold/testsuite/plugin_test_wrap_symbols.sh
+++ b/gold/testsuite/plugin_test_wrap_symbols.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+# plugin_test_wrap_symbols.sh -- a test case for the plugin API.
+
+# Copyright (C) 2017 onwards Free Software Foundation, Inc.
+# Written by Teresa Johnson <tejohnson@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# This file goes with plugin_test.c, a simple plug-in library that
+# exercises the basic interfaces and prints out version numbers and
+# options passed to the plugin.
+
+# This checks if the get_wrap_symbols plugin interface works as
+# expected.
+
+check()
+{
+    if ! grep -q "$2" "$1"
+    then
+	echo "Did not find expected output in $1:"
+	echo "   $2"
+	echo ""
+	echo "Actual output below:"
+	cat "$1"
+	exit 1
+    fi
+}
+
+check plugin_test_wrap_symbols.err "API version:"
+check plugin_test_wrap_symbols.err "gold version:"
+check plugin_test_wrap_symbols.err "Number of wrap symbols = 2"
+check plugin_test_wrap_symbols.err "Wrap symbol hello"
+check plugin_test_wrap_symbols.err "Wrap symbol jello"
+check plugin_test_wrap_symbols.err "cleanup hook called"
+
+exit 0
diff --git a/gold/testsuite/plugin_test_wrap_symbols_1.cc b/gold/testsuite/plugin_test_wrap_symbols_1.cc
index e69de29bb2..90587efbb9 100644
--- a/gold/testsuite/plugin_test_wrap_symbols_1.cc
+++ b/gold/testsuite/plugin_test_wrap_symbols_1.cc
@@ -0,0 +1,40 @@
+/* plugin_test_wrap_symbols -- a test case for gold
+
+   Copyright (C) 2017 onwards Free Software Foundation, Inc.
+   Written by Sriraman Tallam <tmsriram@google.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+extern "C" {
+int __real_hello();
+int __wrap_hello()
+{
+  return __real_hello();
+}
+int hello();
+int __real_jello();
+int __wrap_jello()
+{
+  return __real_jello();
+}
+int jello();
+int main()
+{
+  return 0;
+}
+}
diff --git a/gold/testsuite/plugin_test_wrap_symbols_2.cc b/gold/testsuite/plugin_test_wrap_symbols_2.cc
index e69de29bb2..902cc3c076 100644
--- a/gold/testsuite/plugin_test_wrap_symbols_2.cc
+++ b/gold/testsuite/plugin_test_wrap_symbols_2.cc
@@ -0,0 +1,33 @@
+/* plugin_test_wrap_symbols -- a test case for gold
+
+   Copyright (C) 2017 onwards Free Software Foundation, Inc.
+   Written by Sriraman Tallam <tmsriram@google.com>.
+
+   This file is part of gold.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+   MA 02110-1301, USA.  */
+
+extern "C" {
+int hello()
+{
+  return 0;
+}
+
+int jello()
+{
+  return 0;
+}
+}
diff --git a/include/plugin-api.h b/include/plugin-api.h
index 3a3e8b456d..901369ad4d 100644
--- a/include/plugin-api.h
+++ b/include/plugin-api.h
@@ -365,6 +365,11 @@ enum ld_plugin_status
 (*ld_plugin_get_input_section_size) (const struct ld_plugin_section section,
                                      uint64_t *secsize);
 
+typedef
+enum ld_plugin_status
+(*ld_plugin_get_wrap_symbols) (uint64_t *num_symbols,
+                               const char ***wrap_symbol_list);
+
 enum ld_plugin_level
 {
   LDPL_INFO,
@@ -407,7 +412,8 @@ enum ld_plugin_tag
   LDPT_UNIQUE_SEGMENT_FOR_SECTIONS = 27,
   LDPT_GET_SYMBOLS_V3 = 28,
   LDPT_GET_INPUT_SECTION_ALIGNMENT = 29,
-  LDPT_GET_INPUT_SECTION_SIZE = 30
+  LDPT_GET_INPUT_SECTION_SIZE = 30,
+  LDPT_GET_WRAP_SYMBOLS = 31
 };
 
 /* The plugin transfer vector.  */
@@ -441,6 +447,7 @@ struct ld_plugin_tv
     ld_plugin_unique_segment_for_sections tv_unique_segment_for_sections;
     ld_plugin_get_input_section_alignment tv_get_input_section_alignment;
     ld_plugin_get_input_section_size tv_get_input_section_size;
+    ld_plugin_get_wrap_symbols tv_get_wrap_symbols;
   } tv_u;
 };
 

Re: New gold plugin interface for getting --wrap symbols

[Cary Coutant]

> 2017-11-21  Sriraman Tallam  <tmsriram@google.com>
>
>       * plugin.cc (get_wrap_symbols): New plugin interface.
>       (load): Add get_wrap_symbols to transfer vector.
>       * plugin-api.h (ld_plugin_get_wrap_symbols): New plugin interface.
>       * options.h (options::String_set::size_type): New macro function.

Change this to: (DEFINE_string): Add varname_size() method.

>       * testsuite/plugin_test.c (onload): Call and check get_wrap_symbols
>       interface.
>       * testsuite/plugin_test_wrap_symbols.sh: New test script.
>       * testsuite/plugin_test_wrap_symbols_1.cc: New file.
>       * testsuite/plugin_test_wrap_symbols_2.cc: New file.
>       * testsuite/Makefile.am (plugin_test_wrap_symbols): New test.
>       * testsuite/Makefile.in: Regenerate.

plugin-api.h is in include/ which has its own ChangeLog. That patch
should be applied first and synced with gcc (not sure if they'll take
this during Stage 3, but I think it's harmless enough to be OK). The
gold part is OK after that's checked in.

-cary

Re: New gold plugin interface for getting --wrap symbols

[Sriraman Tallam via binutils]

This patch is now committed and pushed to binutils and I will push
plugin-api.h changes to GCC once Stage1 opens again.

On Fri, Dec 1, 2017 at 4:32 PM, Cary Coutant <ccoutant@gmail.com> wrote:
>> 2017-11-21  Sriraman Tallam  <tmsriram@google.com>
>>
>>       * plugin.cc (get_wrap_symbols): New plugin interface.
>>       (load): Add get_wrap_symbols to transfer vector.
>>       * plugin-api.h (ld_plugin_get_wrap_symbols): New plugin interface.
>>       * options.h (options::String_set::size_type): New macro function.
>
> Change this to: (DEFINE_string): Add varname_size() method.
>
>>       * testsuite/plugin_test.c (onload): Call and check get_wrap_symbols
>>       interface.
>>       * testsuite/plugin_test_wrap_symbols.sh: New test script.
>>       * testsuite/plugin_test_wrap_symbols_1.cc: New file.
>>       * testsuite/plugin_test_wrap_symbols_2.cc: New file.
>>       * testsuite/Makefile.am (plugin_test_wrap_symbols): New test.
>>       * testsuite/Makefile.in: Regenerate.
>
> plugin-api.h is in include/ which has its own ChangeLog. That patch
> should be applied first and synced with gcc (not sure if they'll take
> this during Stage 3, but I think it's harmless enough to be OK). The
> gold part is OK after that's checked in.
>
> -cary