Re: [PATCH 2/2] [GOLD] Add plugin API for processing plugin-added input files

["Sriraman Tallam via binutils" <binutils at sourceware dot org>]

On Mon, Sep 25, 2017 at 6:46 PM, Stephen Crane <sjc@immunant.com> wrote:
> Here is a gold test for the new API. I've included just the test in
> this patch, hopefully that's ok. I wasn't sure if it was better to
> have a separate shell script for the new test or combine it with
> plugin_final_layout.sh since they are testing for the same result. If
> you have any opinions on that, I'm happy to change it.
>
> New test exercises the new input hook by generating a new file using the basic
> test plugin and then reordering its sections and assigning sections to unique
> segments in a second plugin.
>
> gold/ChangeLog:
> 2017-09-25  Stephen Crane  <sjc@immunant.com.com>
>
>     * testsuite/plugin_new_section_layout.c: New plugin to test
>     new_input plugin API.
>     * testsuite/plugin_layout_new_file.sh: New test shell script based
>     on plugin_final_layout.sh.
>     * testsuite/Makefile.am (plugin_layout_new_file): New test case.
>     * testsuite/Makefile.in: Regenerate.
>
> ---
>  gold/testsuite/Makefile.am                 |  19 +++
>  gold/testsuite/Makefile.in                 |  27 ++++-
>  gold/testsuite/plugin_layout_new_file.sh   |  91 +++++++++++++++
>  gold/testsuite/plugin_new_section_layout.c | 181 +++++++++++++++++++++++++++++
>  5 files changed, 324 insertions(+), 3 deletions(-)
>  create mode 100755 gold/testsuite/plugin_layout_new_file.sh
>  create mode 100644 gold/testsuite/plugin_new_section_layout.c
>
> diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
> index 26ee77abcc..e2de96c69b 100644
> --- a/gold/testsuite/Makefile.am
> +++ b/gold/testsuite/Makefile.am
> @@ -2390,6 +2390,25 @@ plugin_section_alignment.so:
> plugin_section_alignment.o gcctestdir/ld
>  plugin_section_alignment.o: plugin_section_alignment.cc
>   $(CXXCOMPILE) -O0 -c -fpic -o $@ $<
>
> +check_SCRIPTS += plugin_layout_new_file.sh
> +check_DATA += plugin_layout_new_file.stdout
> plugin_layout_new_file_readelf.stdout
> +MOSTLYCLEANFILES += plugin_layout_new_file
> +plugin_layout_new_file.o: plugin_final_layout.cc
> + $(CXXCOMPILE) -O0 -c -ffunction-sections  -fdata-sections -g -o $@ $<
> +plugin_layout_new_file.o.syms: plugin_layout_new_file.o
> + $(TEST_READELF) -sW $< >$@ 2>/dev/null
> +plugin_layout_new_file: plugin_layout_new_file.o.syms plugin_test.so
> plugin_new_section_layout.so gcctestdir/ld
> + $(CXXLINK) -Bgcctestdir/ -Wl,--plugin,"./plugin_test.so"
> -Wl,--plugin,"./plugin_new_section_layout.so"
> plugin_layout_new_file.o.syms
> +plugin_layout_new_file.stdout: plugin_layout_new_file
> + $(TEST_NM) -n --synthetic plugin_layout_new_file >
> plugin_layout_new_file.stdout
> +plugin_layout_new_file_readelf.stdout: plugin_layout_new_file
> + $(TEST_READELF) -Wl plugin_layout_new_file >
> plugin_layout_new_file_readelf.stdout
> +
> +plugin_new_section_layout.so: plugin_new_section_layout.o gcctestdir/ld
> + $(LINK) -Bgcctestdir/ -shared plugin_new_section_layout.o
> +plugin_new_section_layout.o: plugin_new_section_layout.c
> + $(COMPILE) -O0 -c -fpic -o $@ $<
> +
>  endif PLUGINS
>
>  check_PROGRAMS += exclude_libs_test
> diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
> index eae68b56cb..9bab136100 100644
> --- a/gold/testsuite/Makefile.in
> +++ b/gold/testsuite/Makefile.in
> @@ -570,12 +570,16 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2)
> $(am__EXEEXT_3) \
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@@TLS_TRUE@am__append_51 =
> plugin_test_tls.err
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_52 = unused.c \
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_final_layout \
> -@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_with_alignment
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_with_alignment \
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_new_file
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_53 =
> plugin_final_layout.sh \
> -@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_with_alignment.sh
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_with_alignment.sh \
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_new_file.sh
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_54 =
> plugin_final_layout.stdout \
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@
> plugin_final_layout_readelf.stdout \
> -@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@
> plugin_layout_with_alignment.stdout
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@
> plugin_layout_with_alignment.stdout \
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_layout_new_file.stdout \
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@
> plugin_layout_new_file_readelf.stdout
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_55 = exclude_libs_test \
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@ local_labels_test \
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@ discard_locals_test
> @@ -5220,6 +5224,8 @@ plugin_final_layout.sh.log: plugin_final_layout.sh
>   @p='plugin_final_layout.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst"
> $(am__check_post)
>  plugin_layout_with_alignment.sh.log: plugin_layout_with_alignment.sh
>   @p='plugin_layout_with_alignment.sh'; $(am__check_pre)
> $(LOG_COMPILE) "$$tst" $(am__check_post)
> +plugin_layout_new_file.sh.log: plugin_layout_new_file.sh
> + @p='plugin_layout_new_file.sh'; $(am__check_pre) $(LOG_COMPILE)
> "$$tst" $(am__check_post)
>  exclude_libs_test.sh.log: exclude_libs_test.sh
>   @p='exclude_libs_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst"
> $(am__check_post)
>  discard_locals_test.sh.log: discard_locals_test.sh
> @@ -6978,6 +6984,21 @@ uninstall-am:
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(CXXLINK) -Bgcctestdir/
> -shared plugin_section_alignment.o
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_section_alignment.o:
> plugin_section_alignment.cc
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(CXXCOMPILE) -O0 -c
> -fpic -o $@ $<
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_layout_new_file.o:
> plugin_final_layout.cc
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(CXXCOMPILE) -O0 -c
> -ffunction-sections  -fdata-sections -g -o $@ $<
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_layout_new_file.o.syms:
> plugin_layout_new_file.o
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(TEST_READELF) -sW $<
>>$@ 2>/dev/null
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_layout_new_file:
> plugin_layout_new_file.o.syms plugin_test.so
> plugin_new_section_layout.so gcctestdir/ld
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(CXXLINK) -Bgcctestdir/
> -Wl,--plugin,"./plugin_test.so"
> -Wl,--plugin,"./plugin_new_section_layout.so"
> plugin_layout_new_file.o.syms
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_layout_new_file.stdout:
> plugin_layout_new_file
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(TEST_NM) -n
> --synthetic plugin_layout_new_file > plugin_layout_new_file.stdout
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_layout_new_file_readelf.stdout:
> plugin_layout_new_file
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(TEST_READELF) -Wl
> plugin_layout_new_file > plugin_layout_new_file_readelf.stdout
> +
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_new_section_layout.so:
> plugin_new_section_layout.o gcctestdir/ld
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(LINK) -Bgcctestdir/
> -shared plugin_new_section_layout.o
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_new_section_layout.o:
> plugin_new_section_layout.c
> +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(COMPILE) -O0 -c -fpic -o $@ $<
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@exclude_libs_test.syms: exclude_libs_test
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -sW $< >$@ 2>/dev/null
>  @GCC_TRUE@@NATIVE_LINKER_TRUE@libexclude_libs_test_1.a: exclude_libs_test_1.o
> diff --git a/gold/testsuite/plugin_layout_new_file.sh
> b/gold/testsuite/plugin_layout_new_file.sh
> new file mode 100755
> index 0000000000..09885e67f0
> --- /dev/null
> +++ b/gold/testsuite/plugin_layout_new_file.sh
> @@ -0,0 +1,91 @@
> +#!/bin/sh
> +
> +# plugin_new_section_layout.sh -- test
> +
> +# Copyright (C) 2011-2017 Free Software Foundation, Inc.
> +# Written by Sriraman Tallam <tmsriram@google.com>.

Copy paste error.

> +
> +# 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.
> +
> +# The goal of this program is to verify if plugin section reordering and
> +# assignment works for plugin-created object files.  File final_layout.cc is in
> +# this test.

again copy paste error.

> +
> +
> +set -e
> +
> +check()
> +{
> +    awk "
> +BEGIN { saw1 = 0; saw2 = 0; err = 0; }
> +/.*$2\$/ { saw1 = 1; }
> +/.*$3\$/ {
> +     saw2 = 1;
> +     if (!saw1)
> +       {
> +   printf \"layout of $2 and $3 is not right\\n\";
> +   err = 1;
> +   exit 1;
> +       }
> +    }
> +END {
> +      if (!saw1 && !err)
> +        {
> +   printf \"did not see $2\\n\";
> +   exit 1;
> + }
> +      if (!saw2 && !err)
> + {
> +   printf \"did not see $3\\n\";
> +   exit 1;
> + }
> +    }" $1
> +}
> +
> +# With readelf -l, an ELF Section to Segment mapping is printed as :
> +##############################################
> +#  Section to Segment mapping:
> +#  Segment Sections...
> +#  ...
> +#     0x     .text.plugin_created_unique
> +#  ...
> +##############################################
> +# Check of .text.plugin_created_unique is the only section in the segment.
> +check_unique_segment()
> +{
> +    awk "
> +BEGIN { saw_section = 0; saw_unique = 0; }
> +/$2/ { saw_section = 1; }
> +/[ ]*0[0-9][ ]*$2[ ]*\$/ { saw_unique = 1; }
> +END {
> +      if (!saw_section)
> + {
> +   printf \"Section $2 not seen in output\\n\";
> +   exit 1;
> + }
> +      else if (!saw_unique)
> + {
> +   printf \"Unique segment not seen for: $2\\n\";
> +   exit 1;
> + }
> +    }" $1
> +}
> +
> +check plugin_layout_new_file.stdout "_Z3foov" "_Z3barv"
> +check plugin_layout_new_file.stdout "_Z3barv" "_Z3bazv"
> +check_unique_segment plugin_layout_new_file_readelf.stdout
> ".text.plugin_created_unique"
> diff --git a/gold/testsuite/plugin_new_section_layout.c
> b/gold/testsuite/plugin_new_section_layout.c
> new file mode 100644
> index 0000000000..f27ad4c530
> --- /dev/null
> +++ b/gold/testsuite/plugin_new_section_layout.c
> @@ -0,0 +1,181 @@
> +/* plugin_new_section_layout.c -- Simple plugin to reorder function sections in
> +   plugin-generated objects
> +
> +   Copyright (C) 2017 Free Software Foundation, Inc.
> +   Written by Stephen Crane <sjc@immunant.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 plugin tests the new_input API of the linker plugin interface that
> + * allows plugins to modify section layout and assign sections to segments for
> + * sections in plugin-generated object files. It assumes that another plugin is
> + * also in use which will add new files. In practice a plugin is likely to
> + * generate new input files itself in all_symbols_read and want to
> + * reorder/assign sections for these files in the new_input_hook callback. */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <assert.h>
> +#include "plugin-api.h"
> +#include "elf/common.h"
> +
> +static ld_plugin_get_input_section_count get_input_section_count = NULL;
> +static ld_plugin_get_input_section_type get_input_section_type = NULL;
> +static ld_plugin_get_input_section_name get_input_section_name = NULL;
> +static ld_plugin_update_section_order update_section_order = NULL;
> +static ld_plugin_allow_section_ordering allow_section_ordering = NULL;
> +static ld_plugin_allow_unique_segment_for_sections
> +    allow_unique_segment_for_sections = NULL;
> +static ld_plugin_unique_segment_for_sections
> unique_segment_for_sections = NULL;
> +
> +enum ld_plugin_status onload(struct ld_plugin_tv *tv);
> +enum ld_plugin_status new_input_hook(const struct ld_plugin_input_file *file);
> +
> +/* Plugin entry point.  */
> +enum ld_plugin_status
> +onload(struct ld_plugin_tv *tv)
> +{
> +  struct ld_plugin_tv *entry;
> +  for (entry = tv; entry->tv_tag != LDPT_NULL; ++entry)
> +    {
> +      switch (entry->tv_tag)
> +        {
> +        case LDPT_GET_INPUT_SECTION_COUNT:
> +          get_input_section_count = *entry->tv_u.tv_get_input_section_count;
> +          break;
> +        case LDPT_GET_INPUT_SECTION_TYPE:
> +          get_input_section_type = *entry->tv_u.tv_get_input_section_type;
> +          break;
> +        case LDPT_GET_INPUT_SECTION_NAME:
> +          get_input_section_name = *entry->tv_u.tv_get_input_section_name;
> +          break;
> + case LDPT_UPDATE_SECTION_ORDER:
> +   update_section_order = *entry->tv_u.tv_update_section_order;
> +   break;
> + case LDPT_ALLOW_SECTION_ORDERING:
> +   allow_section_ordering = *entry->tv_u.tv_allow_section_ordering;
> +   break;
> + case LDPT_ALLOW_UNIQUE_SEGMENT_FOR_SECTIONS:
> +   allow_unique_segment_for_sections
> +       = *entry->tv_u.tv_allow_unique_segment_for_sections;
> +   break;
> + case LDPT_UNIQUE_SEGMENT_FOR_SECTIONS:
> +   unique_segment_for_sections
> +       = *entry->tv_u.tv_unique_segment_for_sections;
> +   break;
> +        case LDPT_REGISTER_NEW_INPUT_HOOK:
> +          assert((*entry->tv_u.tv_register_new_input) (new_input_hook)
> + == LDPS_OK);
> +          break;
> +        default:
> +          break;
> +        }
> +    }
> +
> +  if (get_input_section_count == NULL
> +      || get_input_section_type == NULL
> +      || get_input_section_name == NULL
> +      || update_section_order == NULL
> +      || allow_section_ordering == NULL
> +      || allow_unique_segment_for_sections == NULL
> +      || unique_segment_for_sections == NULL)
> +    {
> +      fprintf(stderr, "Some interfaces are missing\n");
> +      return LDPS_ERR;
> +    }
> +
> +  /* Inform the linker to prepare for section reordering.  */
> +  (*allow_section_ordering)();
> +  /* Inform the linker to prepare to map some sections to unique
> +     segments.  */
> +  (*allow_unique_segment_for_sections)();
> +
> +  return LDPS_OK;
> +}
> +
> +inline static int is_prefix_of(const char *prefix, const char *str)
> +{
> +  return strncmp(prefix, str, strlen (prefix)) == 0;
> +}
> +
> +/* This function is called by the linker when new files are added by a plugin.
> +   We can now tell the linker the desired function order since we have a file
> +   handle for the newly added file.  */
> +
> +enum ld_plugin_status
> +new_input_hook(const struct ld_plugin_input_file *file)
> +{
> +  struct ld_plugin_section section_list[3];
> +  int num_entries = 0;
> +  unsigned int count;
> +
> +  if (get_input_section_count(file->handle, &count) != LDPS_OK)
> +    return LDPS_ERR;
> +
> +  for (unsigned i = 0; i < count; ++i)
> +  {
> +    struct ld_plugin_section section;
> +    unsigned int type = 0;
> +    char *name = NULL;
> +    int position = 3;
> +
> +    section.handle = file->handle;
> +    section.shndx = i;
> +
> +    if (get_input_section_type(section, &type) != LDPS_OK)
> +      return LDPL_FATAL;
> +    if (type != SHT_PROGBITS)
> +      continue;
> +
> +    if (get_input_section_name(section, &name))
> +      return LDPL_FATAL;
> +
> +    /* As in plugin_section_order.c, order is foo() followed by bar()
> +       followed by baz() */
> +    if (is_prefix_of(".text.", name))
> +    {
> +      if (strstr(name, "_Z3foov") != NULL)
> +        position = 0;
> +      else if (strstr(name, "_Z3barv") != NULL)
> +        position = 1;
> +      else if (strstr(name, "_Z3bazv") != NULL)
> +        position = 2;
> +      else
> +        position = 3;
> +    }
> +    if (position < 3)
> +    {
> +      section_list[position] = section;
> +      num_entries++;
> +    }
> +  }
> +
> +  if (num_entries != 3)
> +    return LDPL_FATAL;
> +
> +  update_section_order(section_list, num_entries);
> +  unique_segment_for_sections (".text.plugin_created_unique", 0, 0x1000,
> +                               section_list, num_entries);


It looks fine to me  but I cannot approve.
Thanks
Sri
> +
> +  return LDPS_OK;
> +}
> --
> 2.14.1