This is the mail archive of the
binutils-cvs@sourceware.org
mailing list for the binutils project.
[binutils-gdb] Fix problem where gold fails to issue an undefined symbol error during LTO.
- From: Cary Coutant <ccoutant at sourceware dot org>
- To: bfd-cvs at sourceware dot org
- Date: 22 Mar 2016 04:42:40 -0000
- Subject: [binutils-gdb] Fix problem where gold fails to issue an undefined symbol error during LTO.
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=648c5cbbf34dcbf96bde7e07b14868777fd5d635
commit 648c5cbbf34dcbf96bde7e07b14868777fd5d635
Author: Cary Coutant <ccoutant@gmail.com>
Date: Mon Mar 21 19:07:55 2016 -0700
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.
Diff:
---
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(-)
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 <ccoutant@gmail.com>
+
+ 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 <ccoutant@gmail.com>
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 <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.
+
+#include <iostream>
+
+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 <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.
+
+#include <iostream>
+
+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;
+}