Fix case where IR file provides symbol visibility but replacement file does not.

In PR 22868, two IR files provide conflicting visibility for a symbol.
When a def with PROTECTED visibility is seen after a def with DEFAULT
visibility, gold does not override the visibility. Later, if the
replacement object define the symbol with DEFAULT visibility, the symbol
remains DEFAULT. This was caused by a recent change to allow multiply-defined
absolute symbols, combined with the fact that the plugin framework was using
SHN_ABS as the section index for placeholder symbols. The solution is to
use a real (but arbitrary) section index.

gold/
	PR gold/22868
	* plugin.cc (Sized_pluginobj::do_add_symbols): Use a real section
	index instead of SHN_ABS for defined symbols.
	* testsuite/Makefile.am (plugin_pr22868): New test case.
	* testsuite/Makefile.in: Regenerate
	* testsuite/plugin_pr22868.sh: New test script.
	* testsuite/plugin_pr22868_a.c: New source file.
	* testsuite/plugin_pr22868_b.c: New source file.

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 7364a63bd7d92a75872b3ae5425e431c23630f72..c2613b42371788443985548e6c4cd9a9a5fead36 100644 (file)
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,15 @@
+2018-03-26  Cary Coutant  <ccoutant@gmail.com>
+
+gold/
+       PR gold/22868
+       * plugin.cc (Sized_pluginobj::do_add_symbols): Use a real section
+       index instead of SHN_ABS for defined symbols.
+       * testsuite/Makefile.am (plugin_pr22868): New test case.
+       * testsuite/Makefile.in: Regenerate
+       * testsuite/plugin_pr22868.sh: New test script.
+       * testsuite/plugin_pr22868_a.c: New source file.
+       * testsuite/plugin_pr22868_b.c: New source file.
+
 2018-03-23  Cary Coutant  <ccoutant@gmail.com>
 
        * plugin.cc (link_or_copy_file): Remove newlines from warning messages.

diff --git a/gold/plugin.cc b/gold/plugin.cc
index c921f7ca3d210bbd52d5f54eae7343e631062005..3415b914ad679bbf93a7601ea83c6652c3918334 100644 (file)
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1424,7 +1424,8 @@ Sized_pluginobj<size, big_endian>::do_add_symbols(Symbol_table* symtab,
         {
         case LDPK_DEF:
         case LDPK_WEAKDEF:
-          shndx = elfcpp::SHN_ABS;
+          // We use an arbitrary section number for a defined symbol.
+          shndx = 1;
           break;
         case LDPK_COMMON:
           shndx = elfcpp::SHN_COMMON;

diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index a732b53b4589fec0225a0511cc60b56360ebc3fa..cf0b970eaa1adf2a09351eee1fe7d82a66225974 100644 (file)
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2469,6 +2469,27 @@ 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_pr22868.sh
+check_DATA += plugin_pr22868.stdout
+MOSTLYCLEANFILES += plugin_pr22868.stdout
+plugin_pr22868.stdout: plugin_pr22868.so
+       $(TEST_READELF) -W --dyn-syms $< >$@ 2>/dev/null
+plugin_pr22868.so: plugin_pr22868_a.o.syms plugin_pr22868_b.o.syms plugin_pr22868_b.o plugin_test.so gcctestdir/ld
+       $(LINK) -Bgcctestdir/ -shared -Wl,--plugin,"./plugin_test.so" plugin_pr22868_a.o.syms plugin_pr22868_b.o.syms
+plugin_pr22868_a.o.syms: plugin_pr22868_a.o
+       $(TEST_READELF) -sW $< >$@ 2>/dev/null
+# Generate the .syms file from an alternate version of the original source
+# file, with a "protected" visibility attribute. We'll link with a
+# "replacement" object that does not have that attribute.
+plugin_pr22868_b.o.syms: plugin_pr22868_b_ir.o
+       $(TEST_READELF) -sW $< >$@ 2>/dev/null
+plugin_pr22868_a.o: plugin_pr22868_a.c
+       $(COMPILE) -c -fpic -o $@ $<
+plugin_pr22868_b_ir.o: plugin_pr22868_b.c
+       $(COMPILE) -c -DIR -fpic -o $@ $<
+plugin_pr22868_b.o: plugin_pr22868_b.c
+       $(COMPILE) -c -fpic -o $@ $<
+
 endif PLUGINS
 
 check_PROGRAMS += exclude_libs_test

diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 6f61eae2cdf2b426d9a01536c05a02a4cabfe90b..2f52741d3927cd2d36423a3a4e94707d8a02d917 100644 (file)
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -603,16 +603,19 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @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_new_file \
-@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_pr22868.stdout
 @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_pr22868.sh
 
 # Uses the plugin_final_layout.sh script above to avoid duplication
 @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_new_file.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_layout_new_file_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_pr22868.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
@@ -5353,6 +5356,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_pr22868.sh.log: plugin_pr22868.sh
+       @p='plugin_pr22868.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
@@ -7168,6 +7173,23 @@ 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_pr22868.stdout: plugin_pr22868.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(TEST_READELF) -W --dyn-syms $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868.so: plugin_pr22868_a.o.syms plugin_pr22868_b.o.syms plugin_pr22868_b.o plugin_test.so gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(LINK) -Bgcctestdir/ -shared -Wl,--plugin,"./plugin_test.so" plugin_pr22868_a.o.syms plugin_pr22868_b.o.syms
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868_a.o.syms: plugin_pr22868_a.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(TEST_READELF) -sW $< >$@ 2>/dev/null
+# Generate the .syms file from an alternate version of the original source
+# file, with a "protected" visibility attribute. We'll link with a
+# "replacement" object that does not have that attribute.
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868_b.o.syms: plugin_pr22868_b_ir.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(TEST_READELF) -sW $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868_a.o: plugin_pr22868_a.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -c -fpic -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868_b_ir.o: plugin_pr22868_b.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -c -DIR -fpic -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868_b.o: plugin_pr22868_b.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -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_pr22868.sh b/gold/testsuite/plugin_pr22868.sh
new file mode 100755 (executable)
index 0000000..72f364b
--- /dev/null
+++ b/gold/testsuite/plugin_pr22868.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+# plugin_pr22868.sh -- a test case for the plugin API.
+
+# Copyright (C) 2018 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@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_pr22868_a.c and plugin_pr22868_b.c,
+# which check that if a symbol is declared PROTECTED in any IR file,
+# it will remain PROTECTED in the output even if the replacement file(s)
+# fail to declare it PROTECTED.
+
+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_pr22868.stdout "PROTECTED.*foo"
+
+exit 0

diff --git a/gold/testsuite/plugin_pr22868_a.c b/gold/testsuite/plugin_pr22868_a.c
new file mode 100644 (file)
index 0000000..86a9843
--- /dev/null
+++ b/gold/testsuite/plugin_pr22868_a.c
@@ -0,0 +1,28 @@
+/* plugin_pr22868_a.c -- a test case for the plugin API with GC.
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   Written by Cary Coutant <ccoutant@gmail.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.  */
+
+int foo(int i) __attribute__ (( weak ));
+
+int foo(int i)
+{
+  return i + 1;
+}

diff --git a/gold/testsuite/plugin_pr22868_b.c b/gold/testsuite/plugin_pr22868_b.c
new file mode 100644 (file)
index 0000000..92d2145
--- /dev/null
+++ b/gold/testsuite/plugin_pr22868_b.c
@@ -0,0 +1,39 @@
+/* plugin_pr22868_b_ir.c -- a test case for the plugin API with GC.
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   Written by Cary Coutant <ccoutant@gmail.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 is compiled with -DIR to generate the .syms file,
+   and without -DIR for use as the replacement object.
+   The .syms file declares foo with protected visibility, but
+   the replacement file does not.  */
+
+#ifdef IR
+#define PROTECTED __attribute__ (( visibility ("protected") ))
+#else
+#define PROTECTED
+#endif
+
+int foo(int i) __attribute__ (( weak )) PROTECTED;
+
+int foo(int i)
+{
+  return i + 1;
+}