From 890d155592e66dc01fc4a9affba806c4e9fc36ba Mon Sep 17 00:00:00 2001 From: Cary Coutant Date: Mon, 23 Apr 2018 09:27:35 -0700 Subject: 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. --- 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(-) create mode 100755 gold/testsuite/ver_test_pr16504.sh create mode 100644 gold/testsuite/ver_test_pr16504_a.c create mode 100644 gold/testsuite/ver_test_pr16504_a.script create mode 100644 gold/testsuite/ver_test_pr16504_b.c create mode 100644 gold/testsuite/ver_test_pr16504_b.script (limited to 'gold') diff --git a/gold/ChangeLog b/gold/ChangeLog index bc17a9ea1f8..e38262e294d 100644 --- a/gold/ChangeLog +++ b/gold/ChangeLog @@ -1,3 +1,22 @@ +2018-04-24 Cary Coutant + + 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 PR gold/23046 diff --git a/gold/dynobj.cc b/gold/dynobj.cc index d85adbcbe18..7012802d4fe 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 34551ace7dd..238834dce88 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* ret; + Sized_symbol* 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(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 fdb75114acd..089e156b452 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 7140df64d0d..c926f8ba8fd 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 48e5b9e2fcc..1e6080721cb 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 e80f92626f0..a21abcad4b9 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 00000000000..f4e3b8b0287 --- /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 . + +# 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 00000000000..76280228fad --- /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 00000000000..86bb0a0355c --- /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 00000000000..e6d82b6d37e --- /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 00000000000..94b373dd652 --- /dev/null +++ b/gold/testsuite/ver_test_pr16504_b.script @@ -0,0 +1,4 @@ +VER1 { +global: + foo; +}; -- cgit v1.2.1