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.

7 files changed, 171 insertions, 4 deletions

diff --git a/gold/ChangeLog b/gold/ChangeLog
index e7870207493..891d6a03a26 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -2,6 +2,17 @@
 
 	* testsuite/split_x86_64.sh: Fix bad regexp.
 
+2018-03-26  Cary Coutant  <ccoutant@gmail.com>
+
+	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-02-07  Alan Modra  <amodra@gmail.com>
 
 	Revert 2018-01-17  Alan Modra  <amodra@gmail.com>
diff --git a/gold/plugin.cc b/gold/plugin.cc
index 02fef25dda5..a59f19dcc7b 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1144,7 +1144,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 16cae8004c9..d086dad9c65 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2433,6 +2433,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 bbf8dc146bb..22940cada27 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -591,16 +591,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
@@ -5299,6 +5302,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
@@ -7086,6 +7091,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
index 00000000000..72f364b9a22
--- /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
index 00000000000..86a98432876
--- /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
index 00000000000..92d2145b2d8
--- /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;
+}