From: Cary Coutant Date: Fri, 23 Mar 2018 16:03:34 +0000 (-0700) Subject: Fix case where IR file provides symbol visibility but replacement file does not. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=0b7a4aa6ba5a144b7ce616e80e95d9ff944fec2e;p=binutils-gdb.git 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 7364a63bd7d..c2613b42371 100644 --- a/gold/ChangeLog +++ b/gold/ChangeLog @@ -1,3 +1,15 @@ +2018-03-26 Cary Coutant + +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 * plugin.cc (link_or_copy_file): Remove newlines from warning messages. diff --git a/gold/plugin.cc b/gold/plugin.cc index c921f7ca3d2..3415b914ad6 100644 --- a/gold/plugin.cc +++ b/gold/plugin.cc @@ -1424,7 +1424,8 @@ Sized_pluginobj::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 a732b53b458..cf0b970eaa1 100644 --- 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 6f61eae2cdf..2f52741d392 100644 --- 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 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 . + +# 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 . + + 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 . + + 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; +}