This is the mail archive of the
binutils-cvs@sourceware.org
mailing list for the binutils project.
[binutils-gdb] Fix internal error caused by conflicting default version definitions.
- From: Cary Coutant <ccoutant at sourceware dot org>
- To: bfd-cvs at sourceware dot org
- Date: 24 Apr 2018 20:57:54 -0000
- Subject: [binutils-gdb] Fix internal error caused by conflicting default version definitions.
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=890d155592e66dc01fc4a9affba806c4e9fc36ba
commit 890d155592e66dc01fc4a9affba806c4e9fc36ba
Author: Cary Coutant <ccoutant@gmail.com>
Date: Mon Apr 23 09:27:35 2018 -0700
Fix internal error caused by conflicting default version definitions.
gold/
PR gold/16504
* dynobj.cc (Versions::symbol_section_contents): Don't set
VERSYM_HIDDEN flag for undefined symbols.
* symtab.cc (Symbol_table::add_from_object): Don't override default
version definition with a different default version.
* symtab.h (Symbol::from_dyn): New method.
* testsuite/plugin_test.c (struct sym_info): Add ver field.
(claim_file_hook): Pass symbol version to plugin API.
(parse_readelf_line): Parse symbol version.
* testsuite/Makefile.am (ver_test_pr16504): New test case.
* testsuite/Makefile.in: Regenerate.
* testsuite/ver_test_pr16504.sh: New test script.
* testsuite/ver_test_pr16504_a.c: New source file.
* testsuite/ver_test_pr16504_a.script: New version script.
* testsuite/ver_test_pr16504_b.c: New source file.
* testsuite/ver_test_pr16504_b.script: New version script.
Diff:
---
gold/ChangeLog | 19 +++++++++++++
gold/dynobj.cc | 5 +++-
gold/symtab.cc | 47 ++++++++++++++++++++++++--------
gold/symtab.h | 5 ++++
gold/testsuite/Makefile.am | 17 ++++++++++++
gold/testsuite/Makefile.in | 22 +++++++++++++--
gold/testsuite/plugin_test.c | 39 ++++++++++++++++++++------
gold/testsuite/ver_test_pr16504.sh | 41 ++++++++++++++++++++++++++++
gold/testsuite/ver_test_pr16504_a.c | 5 ++++
gold/testsuite/ver_test_pr16504_a.script | 4 +++
gold/testsuite/ver_test_pr16504_b.c | 10 +++++++
gold/testsuite/ver_test_pr16504_b.script | 4 +++
12 files changed, 196 insertions(+), 22 deletions(-)
diff --git a/gold/ChangeLog b/gold/ChangeLog
index bc17a9e..e38262e 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,22 @@
+2018-04-24 Cary Coutant <ccoutant@gmail.com>
+
+ PR gold/16504
+ * dynobj.cc (Versions::symbol_section_contents): Don't set
+ VERSYM_HIDDEN flag for undefined symbols.
+ * symtab.cc (Symbol_table::add_from_object): Don't override default
+ version definition with a different default version.
+ * symtab.h (Symbol::from_dyn): New method.
+ * testsuite/plugin_test.c (struct sym_info): Add ver field.
+ (claim_file_hook): Pass symbol version to plugin API.
+ (parse_readelf_line): Parse symbol version.
+ * testsuite/Makefile.am (ver_test_pr16504): New test case.
+ * testsuite/Makefile.in: Regenerate.
+ * testsuite/ver_test_pr16504.sh: New test script.
+ * testsuite/ver_test_pr16504_a.c: New source file.
+ * testsuite/ver_test_pr16504_a.script: New version script.
+ * testsuite/ver_test_pr16504_b.c: New source file.
+ * testsuite/ver_test_pr16504_b.script: New version script.
+
2018-04-19 Cary Coutant <ccoutant@gmail.com>
PR gold/23046
diff --git a/gold/dynobj.cc b/gold/dynobj.cc
index d85adbc..7012802 100644
--- a/gold/dynobj.cc
+++ b/gold/dynobj.cc
@@ -1776,7 +1776,10 @@ Versions::symbol_section_contents(const Symbol_table* symtab,
version_index = this->version_index(symtab, dynpool, *p);
// If the symbol was defined as foo@V1 instead of foo@@V1, add
// the hidden bit.
- if ((*p)->version() != NULL && !(*p)->is_default())
+ if ((*p)->version() != NULL
+ && (*p)->is_defined()
+ && !(*p)->is_default()
+ && !(*p)->from_dyn())
version_index |= elfcpp::VERSYM_HIDDEN;
elfcpp::Swap<16, big_endian>::writeval(pbuf + (*p)->dynsym_index() * 2,
version_index);
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 34551ac..238834d 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -989,7 +989,7 @@ Symbol_table::add_from_object(Object* object,
// ins.first->second: the value (Symbol*).
// ins.second: true if new entry was inserted, false if not.
- Sized_symbol<size>* ret;
+ Sized_symbol<size>* ret = NULL;
bool was_undefined_in_reg;
bool was_common;
if (!ins.second)
@@ -1049,17 +1049,42 @@ Symbol_table::add_from_object(Object* object,
// it, then change it to NAME/VERSION.
ret = this->get_sized_symbol<size>(insdefault.first->second);
- was_undefined_in_reg = ret->is_undefined() && ret->in_reg();
- // Commons from plugins are just placeholders.
- was_common = ret->is_common() && ret->object()->pluginobj() == NULL;
-
- this->resolve(ret, sym, st_shndx, is_ordinary, orig_st_shndx, object,
- version, is_default_version);
- if (parameters->options().gc_sections())
- this->gc_mark_dyn_syms(ret);
- ins.first->second = ret;
+ // If the existing symbol already has a version,
+ // don't override it with the new symbol.
+ // This should only happen when the new symbol
+ // is from a shared library.
+ if (ret->version() != NULL)
+ {
+ if (!object->is_dynamic())
+ {
+ gold_warning(_("%s: conflicting default version definition"
+ " for %s@@%s"),
+ object->name().c_str(), name, version);
+ if (ret->source() == Symbol::FROM_OBJECT)
+ gold_info(_("%s: %s: previous definition of %s@@%s here"),
+ program_name,
+ ret->object()->name().c_str(),
+ name, ret->version());
+ }
+ ret = NULL;
+ is_default_version = false;
+ }
+ else
+ {
+ was_undefined_in_reg = ret->is_undefined() && ret->in_reg();
+ // Commons from plugins are just placeholders.
+ was_common = (ret->is_common()
+ && ret->object()->pluginobj() == NULL);
+
+ this->resolve(ret, sym, st_shndx, is_ordinary, orig_st_shndx,
+ object, version, is_default_version);
+ if (parameters->options().gc_sections())
+ this->gc_mark_dyn_syms(ret);
+ ins.first->second = ret;
+ }
}
- else
+
+ if (ret == NULL)
{
was_undefined_in_reg = false;
was_common = false;
diff --git a/gold/symtab.h b/gold/symtab.h
index fdb7511..089e156 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -344,6 +344,11 @@ class Symbol
set_in_dyn()
{ this->in_dyn_ = true; }
+ // Return whether this symbol is defined in a dynamic object.
+ bool
+ from_dyn() const
+ { return this->source_ == FROM_OBJECT && this->object()->is_dynamic(); }
+
// Return whether this symbol has been seen in a real ELF object.
// (IN_REG will return TRUE if the symbol has been seen in either
// a real ELF object or an object claimed by a plugin.)
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 7140df6..c926f8b 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2516,6 +2516,23 @@ plugin_pr22868_b_ir.o: plugin_pr22868_b.c
plugin_pr22868_b.o: plugin_pr22868_b.c
$(COMPILE) -c -fpic -o $@ $<
+check_SCRIPTS += ver_test_pr16504.sh
+check_DATA += ver_test_pr16504.stdout
+ver_test_pr16504.stdout: ver_test_pr16504.so
+ $(TEST_READELF) -W --dyn-syms $< >$@ 2>/dev/null
+ver_test_pr16504.so: ver_test_pr16504_a.so ver_test_pr16504_b.o.syms ver_test_pr16504_b.script gcctestdir/ld
+ gcctestdir/ld -shared -o $@ --plugin ./plugin_test.so --version-script $(srcdir)/ver_test_pr16504_b.script ver_test_pr16504_b.o.syms ver_test_pr16504_a.so
+ver_test_pr16504_a.so: ver_test_pr16504_a.o ver_test_pr16504_a.script gcctestdir/ld
+ gcctestdir/ld -shared -o $@ --version-script $(srcdir)/ver_test_pr16504_a.script ver_test_pr16504_a.o
+ver_test_pr16504_a.o: ver_test_pr16504_a.c
+ $(COMPILE) -c -fpic -o $@ $<
+# Filter out the UNDEFs from the symbols file to simulate GCC behavior,
+# which does not pass the UNDEF from a .symver directive.
+ver_test_pr16504_b.o.syms: ver_test_pr16504_b.o
+ $(TEST_READELF) -sW $< 2>/dev/null | grep -v "UND" >$@
+ver_test_pr16504_b.o: ver_test_pr16504_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 48e5b9e..1e60807 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -612,7 +612,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
@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_pr22868.sh
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_pr22868.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ ver_test_pr16504.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 \
@@ -620,7 +621,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
@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_pr22868.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ plugin_pr22868.stdout \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ ver_test_pr16504.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
@@ -5368,6 +5370,8 @@ 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)
+ver_test_pr16504.sh.log: ver_test_pr16504.sh
+ @p='ver_test_pr16504.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
@@ -7220,6 +7224,20 @@ uninstall-am:
@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@@PLUGINS_TRUE@ver_test_pr16504.stdout: ver_test_pr16504.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(TEST_READELF) -W --dyn-syms $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504.so: ver_test_pr16504_a.so ver_test_pr16504_b.o.syms ver_test_pr16504_b.script gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ gcctestdir/ld -shared -o $@ --plugin ./plugin_test.so --version-script $(srcdir)/ver_test_pr16504_b.script ver_test_pr16504_b.o.syms ver_test_pr16504_a.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_a.so: ver_test_pr16504_a.o ver_test_pr16504_a.script gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ gcctestdir/ld -shared -o $@ --version-script $(srcdir)/ver_test_pr16504_a.script ver_test_pr16504_a.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_a.o: ver_test_pr16504_a.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(COMPILE) -c -fpic -o $@ $<
+# Filter out the UNDEFs from the symbols file to simulate GCC behavior,
+# which does not pass the UNDEF from a .symver directive.
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_b.o.syms: ver_test_pr16504_b.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ $(TEST_READELF) -sW $< 2>/dev/null | grep -v "UND" >$@
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_b.o: ver_test_pr16504_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_test.c b/gold/testsuite/plugin_test.c
index e80f926..a21abca 100644
--- a/gold/testsuite/plugin_test.c
+++ b/gold/testsuite/plugin_test.c
@@ -46,6 +46,7 @@ struct sym_info
char* vis;
char* sect;
char* name;
+ char* ver;
};
static struct claimed_file* first_claimed_file = NULL;
@@ -397,7 +398,14 @@ claim_file_hook (const struct ld_plugin_input_file* file, int* claimed)
syms[nsyms].name = malloc(len + 1);
strncpy(syms[nsyms].name, info.name, len + 1);
}
- syms[nsyms].version = NULL;
+ if (info.ver == NULL)
+ syms[nsyms].version = NULL;
+ else
+ {
+ len = strlen(info.ver);
+ syms[nsyms].version = malloc(len + 1);
+ strncpy(syms[nsyms].version, info.ver, len + 1);
+ }
syms[nsyms].def = def;
syms[nsyms].visibility = vis;
syms[nsyms].size = info.size;
@@ -676,11 +684,26 @@ parse_readelf_line(char* p, struct sym_info* info)
p += strspn(p, " ");
/* Name field. */
- /* FIXME: Look for version. */
- len = strlen(p);
- if (len == 0)
- p = NULL;
- else if (p[len-1] == '\n')
- p[--len] = '\0';
- info->name = p;
+ len = strcspn(p, "@\n");
+ if (len > 0 && p[len] == '@')
+ {
+ /* Get the symbol version. */
+ char* vp = p + len;
+ int vlen;
+
+ vp += strspn(vp, "@");
+ vlen = strcspn(vp, "\n");
+ vp[vlen] = '\0';
+ if (vlen > 0)
+ info->ver = vp;
+ else
+ info->ver = NULL;
+ }
+ else
+ info->ver = NULL;
+ p[len] = '\0';
+ if (len > 0)
+ info->name = p;
+ else
+ info->name = NULL;
}
diff --git a/gold/testsuite/ver_test_pr16504.sh b/gold/testsuite/ver_test_pr16504.sh
new file mode 100755
index 0000000..f4e3b8b
--- /dev/null
+++ b/gold/testsuite/ver_test_pr16504.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+# ver_test_pr16504.sh -- test that undef gets correct version with LTO.
+
+# 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.
+
+check()
+{
+ if ! grep -q "$2" "$1"
+ then
+ echo "Did not find expected symbol in $1:"
+ echo " $2"
+ echo ""
+ echo "Actual output below:"
+ cat "$1"
+ exit 1
+ fi
+}
+
+check ver_test_pr16504.stdout " FUNC .* UND *foo@VER2"
+check ver_test_pr16504.stdout " IFUNC .* foo@@VER1"
+
+exit 0
diff --git a/gold/testsuite/ver_test_pr16504_a.c b/gold/testsuite/ver_test_pr16504_a.c
new file mode 100644
index 0000000..7628022
--- /dev/null
+++ b/gold/testsuite/ver_test_pr16504_a.c
@@ -0,0 +1,5 @@
+const char* foo(void);
+
+const char*
+foo(void)
+{ return "x"; }
diff --git a/gold/testsuite/ver_test_pr16504_a.script b/gold/testsuite/ver_test_pr16504_a.script
new file mode 100644
index 0000000..86bb0a0
--- /dev/null
+++ b/gold/testsuite/ver_test_pr16504_a.script
@@ -0,0 +1,4 @@
+VER2 {
+global:
+ foo;
+};
diff --git a/gold/testsuite/ver_test_pr16504_b.c b/gold/testsuite/ver_test_pr16504_b.c
new file mode 100644
index 0000000..e6d82b6
--- /dev/null
+++ b/gold/testsuite/ver_test_pr16504_b.c
@@ -0,0 +1,10 @@
+void
+new_foo(void);
+__asm__(".symver new_foo,foo@VER2");
+
+static void (*resolve_foo(void)) (void)
+{
+ return new_foo;
+}
+
+void foo(void) __attribute__((ifunc("resolve_foo")));
diff --git a/gold/testsuite/ver_test_pr16504_b.script b/gold/testsuite/ver_test_pr16504_b.script
new file mode 100644
index 0000000..94b373d
--- /dev/null
+++ b/gold/testsuite/ver_test_pr16504_b.script
@@ -0,0 +1,4 @@
+VER1 {
+global:
+ foo;
+};