From 648c5cbbf34dcbf96bde7e07b14868777fd5d635 Mon Sep 17 00:00:00 2001 From: Cary Coutant Date: Mon, 21 Mar 2016 19:07:55 -0700 Subject: [PATCH] Fix problem where gold fails to issue an undefined symbol error during LTO. During LTO, if (1) an IR file contains a COMDAT group that is kept, (2) a later non-claimed file contains the same group, which we discard, and (3) the plugin fails to provide a definition of the symbols in that COMDAT group, gold silently resolves any references to those symbols to 0. This patch adds a check for a placeholder symbol when deciding whether to issue an undefined symbol error. It also adds an extra note after any undefined placeholder symbol error that explains that a definition was expected from the plugin. gold/ PR gold/19842 * errors.cc (Errors::undefined_symbol): Add info message when symbol should have been provided by a plugin. * target-reloc.h (issue_undefined_symbol_error): Check for placeholder symbols defined in discarded sections. * testsuite/Makefile.am (plugin_test_9b): New test case. * testsuite/Makefile.in: Regenerate. * testsuite/plugin_test_9b_elf.cc: New test source file. * testsuite/plugin_test_9b_ir.cc: New test source file. --- gold/ChangeLog | 12 ++++++++++ gold/errors.cc | 3 +++ gold/target-reloc.h | 6 +++-- gold/testsuite/Makefile.am | 19 +++++++++++++++ gold/testsuite/Makefile.in | 19 +++++++++++++++ gold/testsuite/plugin_test_9b_elf.cc | 40 +++++++++++++++++++++++++++++++ gold/testsuite/plugin_test_9b_ir.cc | 46 ++++++++++++++++++++++++++++++++++++ 7 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 gold/testsuite/plugin_test_9b_elf.cc create mode 100644 gold/testsuite/plugin_test_9b_ir.cc diff --git a/gold/ChangeLog b/gold/ChangeLog index 854af4b..0052c0b 100644 --- a/gold/ChangeLog +++ b/gold/ChangeLog @@ -1,3 +1,15 @@ +2016-03-21 Cary Coutant + + PR gold/19842 + * errors.cc (Errors::undefined_symbol): Add info message when + symbol should have been provided by a plugin. + * target-reloc.h (issue_undefined_symbol_error): Check for + placeholder symbols defined in discarded sections. + * testsuite/Makefile.am (plugin_test_9b): New test case. + * testsuite/Makefile.in: Regenerate. + * testsuite/plugin_test_9b_elf.cc: New test source file. + * testsuite/plugin_test_9b_ir.cc: New test source file. + 2016-03-20 Cary Coutant PR gold/19002 diff --git a/gold/errors.cc b/gold/errors.cc index f90e53f..27392cc 100644 --- a/gold/errors.cc +++ b/gold/errors.cc @@ -198,6 +198,9 @@ Errors::undefined_symbol(const Symbol* sym, const std::string& location) gold_info(_("%s: the vtable symbol may be undefined because " "the class is missing its key function"), program_name); + if (sym->is_placeholder()) + gold_info(_("%s: the symbol should have been defined by a plugin"), + program_name); } // Issue a debugging message. diff --git a/gold/target-reloc.h b/gold/target-reloc.h index bdf673d..e275d33 100644 --- a/gold/target-reloc.h +++ b/gold/target-reloc.h @@ -186,8 +186,10 @@ issue_undefined_symbol_error(const Symbol* sym) if (sym->is_weak_undefined()) return false; - // We don't report symbols defined in discarded sections. - if (sym->is_defined_in_discarded_section()) + // We don't report symbols defined in discarded sections, + // unless they're placeholder symbols that should have been + // provided by a plugin. + if (sym->is_defined_in_discarded_section() && !sym->is_placeholder()) return false; // If the target defines this symbol, don't report it here. diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index fbaee16..bf222c3 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -2015,6 +2015,25 @@ MOSTLYCLEANFILES += two_file_test_1c.o two_file_test_1c.o: two_file_test_1.o cp two_file_test_1.o $@ +# As above, but check COMDAT case, where a non-IR file contains a duplicate +# of a COMDAT group in an IR file. +check_DATA += plugin_test_9b.err +MOSTLYCLEANFILES += plugin_test_9b.err +plugin_test_9b.err: plugin_test_9b_ir.o.syms plugin_test_9b_ir.o plugin_test_9b_elf.o gcctestdir/ld plugin_test.so + @echo $(CXXLINK) -Bgcctestdir/ -o plugin_test_9b -Wl,--no-demangle,--plugin,"./plugin_test.so",--plugin-opt,"_ZN1A5printEv" plugin_test_9b_ir.o plugin_test_9b_elf.o "2>$@" + @if $(CXXLINK) -Bgcctestdir/ -o plugin_test_9b -Wl,--no-demangle,--plugin,"./plugin_test.so",--plugin-opt,"_ZN1A5printEv" plugin_test_9b_ir.o plugin_test_9b_elf.o 2>$@; then \ + echo 1>&2 "Link of plugin_test_9b should have failed"; \ + rm -f $@; \ + exit 1; \ + fi +# Make a .syms file that claims to define a method in class A in a COMDAT group. +# The real plugin_test_9b_ir.o will be compiled without the -D, and will not +# define any methods in class A. +plugin_test_9b_ir.o.syms: plugin_test_9b_ir_witha.o + $(TEST_READELF) -sW $< >$@ 2>/dev/null +plugin_test_9b_ir_witha.o: plugin_test_9b_ir.cc + $(CXXCOMPILE) -c -DUSE_CLASS_A -o $@ $< + check_PROGRAMS += plugin_test_10 check_SCRIPTS += plugin_test_10.sh check_DATA += plugin_test_10.sections diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in index 1246504..43f8465 100644 --- a/gold/testsuite/Makefile.in +++ b/gold/testsuite/Makefile.in @@ -452,6 +452,9 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ # Test that symbols known in the IR file but not in the replacement file # produce an unresolved symbol error. + +# As above, but check COMDAT case, where a non-IR file contains a duplicate +# of a COMDAT group in an IR file. @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_39 = \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_1.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_2.err \ @@ -461,6 +464,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_7.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_7.o.syms \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_9.err \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_9b.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_10.sections \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_11.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_start_lib.err @@ -475,6 +479,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_7.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_9.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ two_file_test_1c.o \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_9b.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_10.sections \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_11.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_test_thin.a \ @@ -6089,6 +6094,20 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ mv -f $@.tmp $@ @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@two_file_test_1c.o: two_file_test_1.o @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ cp two_file_test_1.o $@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_9b.err: plugin_test_9b_ir.o.syms plugin_test_9b_ir.o plugin_test_9b_elf.o gcctestdir/ld plugin_test.so +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -o plugin_test_9b -Wl,--no-demangle,--plugin,"./plugin_test.so",--plugin-opt,"_ZN1A5printEv" plugin_test_9b_ir.o plugin_test_9b_elf.o "2>$@" +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -o plugin_test_9b -Wl,--no-demangle,--plugin,"./plugin_test.so",--plugin-opt,"_ZN1A5printEv" plugin_test_9b_ir.o plugin_test_9b_elf.o 2>$@; then \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ echo 1>&2 "Link of plugin_test_9b should have failed"; \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ rm -f $@; \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ exit 1; \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ fi +# Make a .syms file that claims to define a method in class A in a COMDAT group. +# The real plugin_test_9b_ir.o will be compiled without the -D, and will not +# define any methods in class A. +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_9b_ir.o.syms: plugin_test_9b_ir_witha.o +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(TEST_READELF) -sW $< >$@ 2>/dev/null +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_9b_ir_witha.o: plugin_test_9b_ir.cc +@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(CXXCOMPILE) -c -DUSE_CLASS_A -o $@ $< @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_10: plugin_common_test_1.o.syms plugin_common_test_2.o gcctestdir/ld plugin_test.so @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(CXXLINK) -Bgcctestdir/ -Wl,--no-demangle,--plugin,"./plugin_test.so" plugin_common_test_1.o.syms plugin_common_test_2.o @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_test_10.sections: plugin_test_10 diff --git a/gold/testsuite/plugin_test_9b_elf.cc b/gold/testsuite/plugin_test_9b_elf.cc new file mode 100644 index 0000000..c7a1755 --- /dev/null +++ b/gold/testsuite/plugin_test_9b_elf.cc @@ -0,0 +1,40 @@ +// plugin_test_9b_ir.cc -- a test case for gold plugins + +// Copyright (C) 2016 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. + +#include + +using namespace std; + +class A { + public: + virtual void + print() __attribute__ ((noinline)) + { cout << "A::print" << endl; } +}; + +void +foo() +{ + A a; + a.print(); + cout << "foo returning" << endl; +} diff --git a/gold/testsuite/plugin_test_9b_ir.cc b/gold/testsuite/plugin_test_9b_ir.cc new file mode 100644 index 0000000..a45656d --- /dev/null +++ b/gold/testsuite/plugin_test_9b_ir.cc @@ -0,0 +1,46 @@ +// plugin_test_9b_ir.cc -- a test case for gold plugins + +// Copyright (C) 2016 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. + +#include + +using namespace std; + +class A { + public: + virtual void + print() __attribute__ ((noinline)) + { cout << "A::print" << endl; } +}; + +extern void foo(); + +int +main() +{ +#ifdef USE_CLASS_A + A a; + a.print(); +#endif + cout << "calling foo" << endl; + foo(); + return 0; +} -- 2.7.4