From 89d801850ac03d0531f7eef3d6978bec17a30a2d Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Fri, 31 May 2024 21:30:34 -0700 Subject: gold: Properly remove the versioned symbol When the versioned symbol foo is removed from the shared library, the ".symver foo,foo@VER" directive provides binary compatibility for foo@VER. In this case, the unversioned symbol foo should be hidden and shouldn't generate a multiple definition error. PR gold/31830 * resolve.cc (Symbol_table::resolve): Move symbol version handling to ... * symtab.cc (Symbol_table::add_from_object): Here. If the hidden version from .symver is the same as the default version from the unversioned symbol, don't make the unversioned symbol the default versioned symbol. * testsuite/Makefile.am (check_SCRIPTS): Add ver_test_pr31830.sh. (check_DATA): ver_test_pr31830_a.syms and ver_test_pr31830_b.syms. (ver_test_pr31830_a.syms): New. (ver_test_pr31830_b.syms): Likewise. (ver_test_pr31830_a.so): Likewise. (ver_test_pr31830_b.so): Likewise. * testsuite/Makefile.in: Regenerated. * testsuite/ver_test_pr31830.script: New file. * testsuite/ver_test_pr31830.sh: Likewise. * testsuite/ver_test_pr31830_a.c: Likewise. * testsuite/ver_test_pr31830_b.c: Likewise. * testsuite/ver_test_pr31830_lto.c: Likewise. * testsuite/ver_test_pr31830_lto.sh: Likewise. Signed-off-by: H.J. Lu --- gold/resolve.cc | 14 ------- gold/symtab.cc | 76 +++++++++++++++++++++++++++++----- gold/testsuite/Makefile.am | 22 ++++++++++ gold/testsuite/Makefile.in | 36 ++++++++++++++++ gold/testsuite/ver_test_pr31830.script | 6 +++ gold/testsuite/ver_test_pr31830.sh | 61 +++++++++++++++++++++++++++ gold/testsuite/ver_test_pr31830_a.c | 2 + gold/testsuite/ver_test_pr31830_b.c | 3 ++ gold/testsuite/ver_test_pr31830_lto.c | 5 +++ gold/testsuite/ver_test_pr31830_lto.sh | 61 +++++++++++++++++++++++++++ 10 files changed, 261 insertions(+), 25 deletions(-) create mode 100644 gold/testsuite/ver_test_pr31830.script create mode 100755 gold/testsuite/ver_test_pr31830.sh create mode 100644 gold/testsuite/ver_test_pr31830_a.c create mode 100644 gold/testsuite/ver_test_pr31830_b.c create mode 100644 gold/testsuite/ver_test_pr31830_lto.c create mode 100755 gold/testsuite/ver_test_pr31830_lto.sh (limited to 'gold') diff --git a/gold/resolve.cc b/gold/resolve.cc index 777405d..f055895 100644 --- a/gold/resolve.cc +++ b/gold/resolve.cc @@ -250,20 +250,6 @@ Symbol_table::resolve(Sized_symbol* to, bool to_is_ordinary; const unsigned int to_shndx = to->shndx(&to_is_ordinary); - // It's possible for a symbol to be defined in an object file - // using .symver to give it a version, and for there to also be - // a linker script giving that symbol the same version. We - // don't want to give a multiple-definition error for this - // harmless redefinition. - if (to->source() == Symbol::FROM_OBJECT - && to->object() == object - && to->is_defined() - && is_ordinary - && to_is_ordinary - && to_shndx == st_shndx - && to->value() == sym.get_st_value()) - return; - // Likewise for an absolute symbol defined twice with the same value. if (!is_ordinary && st_shndx == elfcpp::SHN_ABS diff --git a/gold/symtab.cc b/gold/symtab.cc index 9a55e6e..5857dd7 100644 --- a/gold/symtab.cc +++ b/gold/symtab.cc @@ -998,12 +998,64 @@ Symbol_table::add_from_object(Object* object, ret = this->get_sized_symbol(ins.first->second); gold_assert(ret != NULL); + bool ret_is_ordinary; + const unsigned int ret_shndx = ret->shndx(&ret_is_ordinary); + 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); + // It's possible for a symbol to be defined in an object file + // using .symver to give it a version, and for there to also be + // a linker script giving that symbol the same version. We + // don't want to give a multiple-definition error for this + // harmless redefinition. + bool check_version = false; + bool erase_default_version = false; + bool no_default_version = false; + if (ret->source() == Symbol::FROM_OBJECT + && is_ordinary + && ret_shndx == st_shndx) + { + if (ret->object() == object) + check_version = true; + + if (version != NULL && version == ret->version()) + { + // Don't give a multiple-definition error if the hidden + // version from .symver is the same as the default version + // from the unversioned symbol. + if (is_default_version && !ret->is_default ()) + { + no_default_version = true; + if (insdefault.second) + { + // Don't make the unversioned symbol the default + // version. + is_default_version = false; + erase_default_version = true; + check_version = true; + } + } + else if (!is_default_version && ret->is_default ()) + { + // Don't make the unversioned symbol the default + // version. + ret->set_is_not_default(); + no_default_version = true; + check_version = true; + } + } + } + + if (!(check_version + && ret->is_defined() + && ret_is_ordinary + && (no_default_version + || ret->value() == sym.get_st_value()))) + 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); @@ -1012,13 +1064,7 @@ Symbol_table::add_from_object(Object* object, insdefault.first); else { - bool dummy; - if (version != NULL - && ret->source() == Symbol::FROM_OBJECT - && ret->object() == object - && is_ordinary - && ret->shndx(&dummy) == st_shndx - && ret->is_default()) + if (version != NULL && check_version) { // We have seen NAME/VERSION already, and marked it as the // default version, but now we see a definition for @@ -1032,9 +1078,17 @@ Symbol_table::add_from_object(Object* object, // In any other case, the two symbols should have generated // a multiple definition error. // (See PR gold/18703.) - ret->set_is_not_default(); + // If the hidden version from .symver is the same as the + // default version from the unversioned symbol, don't make + // the unversioned symbol the default versioned symbol. const Stringpool::Key vnull_key = 0; - this->table_.erase(std::make_pair(name_key, vnull_key)); + if (erase_default_version) + this->table_.erase(std::make_pair(name_key, vnull_key)); + else if (ret->object() == object) + { + ret->set_is_not_default(); + this->table_.erase(std::make_pair(name_key, vnull_key)); + } } } } diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am index 0685e91..2f1348f 100644 --- a/gold/testsuite/Makefile.am +++ b/gold/testsuite/Makefile.am @@ -1963,6 +1963,28 @@ ver_test_pr23409_1.so: gcctestdir/ld ver_test_1.o $(srcdir)/ver_test_pr23409_1.s ver_test_pr23409_2.so: gcctestdir/ld ver_test_1.o $(srcdir)/ver_test_pr23409_2.script gcctestdir/ld -shared -o $@ ver_test_1.o --version-script $(srcdir)/ver_test_pr23409_2.script +check_SCRIPTS += ver_test_pr31830.sh +check_DATA += ver_test_pr31830_a.syms ver_test_pr31830_b.syms +ver_test_pr31830_a.syms: ver_test_pr31830_a.so + $(TEST_READELF) --dyn-syms -W $< >$@ +ver_test_pr31830_b.syms: ver_test_pr31830_b.so + $(TEST_READELF) --dyn-syms -W $< >$@ +ver_test_pr31830_a.so: gcctestdir/ld ver_test_pr31830_a.o ver_test_pr31830_b.o $(srcdir)/ver_test_pr31830.script + gcctestdir/ld -shared -o $@ ver_test_pr31830_a.o ver_test_pr31830_b.o --version-script $(srcdir)/ver_test_pr31830.script +ver_test_pr31830_b.so: gcctestdir/ld ver_test_pr31830_a.o ver_test_pr31830_b.o $(srcdir)/ver_test_pr31830.script + gcctestdir/ld -shared -o $@ ver_test_pr31830_b.o ver_test_pr31830_a.o --version-script $(srcdir)/ver_test_pr31830.script + +check_SCRIPTS += ver_test_pr31830_lto.sh +check_DATA += ver_test_pr31830_lto_a.syms ver_test_pr31830_lto_b.syms +ver_test_pr31830_lto_a.syms: ver_test_pr31830_lto_a.so + $(TEST_READELF) --dyn-syms -W $< >$@ +ver_test_pr31830_lto_a.so: gcctestdir/ld $(srcdir)/ver_test_pr31830_lto.c $(srcdir)/ver_test_pr31830.script + $(LINK) -Bgcctestdir/ -shared -o $@ -O2 -fPIC -flto-partition=max $(srcdir)/ver_test_pr31830_lto.c -Wl,--version-script,$(srcdir)/ver_test_pr31830.script +ver_test_pr31830_lto_b.syms: ver_test_pr31830_lto_b.so + $(TEST_READELF) --dyn-syms -W $< >$@ +ver_test_pr31830_lto_b.so: gcctestdir/ld $(srcdir)/ver_test_pr31830_lto.c $(srcdir)/ver_test_pr31830.script + $(LINK) -Bgcctestdir/ -shared -o $@ -O2 -fPIC -flto-partition=max -flto=2 $(srcdir)/ver_test_pr31830_lto.c -Wl,--version-script,$(srcdir)/ver_test_pr31830.script + check_SCRIPTS += weak_as_needed.sh check_DATA += weak_as_needed.stdout weak_as_needed.stdout: weak_as_needed_a.so diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in index 33d5cd7..9cf21df 100644 --- a/gold/testsuite/Makefile.in +++ b/gold/testsuite/Makefile.in @@ -469,6 +469,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_10.sh ver_test_13.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_14.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr23409.sh \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr31830.sh \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr31830_lto.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ weak_as_needed.sh relro_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_matching_test.sh \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_3.sh \ @@ -526,6 +528,10 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_13.syms \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_14.syms \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr23409.syms \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr31830_a.syms \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr31830_b.syms \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr31830_lto_a.syms \ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_pr31830_lto_b.syms \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ weak_as_needed.stdout \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ protected_3.err \ @GCC_TRUE@@NATIVE_LINKER_TRUE@ relro_test.stdout \ @@ -5900,6 +5906,20 @@ ver_test_pr23409.sh.log: ver_test_pr23409.sh --log-file $$b.log --trs-file $$b.trs \ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ "$$tst" $(AM_TESTS_FD_REDIRECT) +ver_test_pr31830.sh.log: ver_test_pr31830.sh + @p='ver_test_pr31830.sh'; \ + b='ver_test_pr31830.sh'; \ + $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ + --log-file $$b.log --trs-file $$b.trs \ + $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ + "$$tst" $(AM_TESTS_FD_REDIRECT) +ver_test_pr31830_lto.sh.log: ver_test_pr31830_lto.sh + @p='ver_test_pr31830_lto.sh'; \ + b='ver_test_pr31830_lto.sh'; \ + $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ + --log-file $$b.log --trs-file $$b.trs \ + $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ + "$$tst" $(AM_TESTS_FD_REDIRECT) weak_as_needed.sh.log: weak_as_needed.sh @p='weak_as_needed.sh'; \ b='weak_as_needed.sh'; \ @@ -8900,6 +8920,22 @@ uninstall-am: @GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -o $@ ver_test_1.o ver_test_pr23409_2.so --version-script $(srcdir)/ver_test_pr23409_1.script @GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr23409_2.so: gcctestdir/ld ver_test_1.o $(srcdir)/ver_test_pr23409_2.script @GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -o $@ ver_test_1.o --version-script $(srcdir)/ver_test_pr23409_2.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_a.syms: ver_test_pr31830_a.so +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) --dyn-syms -W $< >$@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_b.syms: ver_test_pr31830_b.so +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) --dyn-syms -W $< >$@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_a.so: gcctestdir/ld ver_test_pr31830_a.o ver_test_pr31830_b.o $(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -o $@ ver_test_pr31830_a.o ver_test_pr31830_b.o --version-script $(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_b.so: gcctestdir/ld ver_test_pr31830_a.o ver_test_pr31830_b.o $(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -shared -o $@ ver_test_pr31830_b.o ver_test_pr31830_a.o --version-script $(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_lto_a.syms: ver_test_pr31830_lto_a.so +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) --dyn-syms -W $< >$@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_lto_a.so: gcctestdir/ld $(srcdir)/ver_test_pr31830_lto.c $(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(LINK) -Bgcctestdir/ -shared -o $@ -O2 -fPIC -flto-partition=max $(srcdir)/ver_test_pr31830_lto.c -Wl,--version-script,$(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_lto_b.syms: ver_test_pr31830_lto_b.so +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) --dyn-syms -W $< >$@ +@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_lto_b.so: gcctestdir/ld $(srcdir)/ver_test_pr31830_lto.c $(srcdir)/ver_test_pr31830.script +@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(LINK) -Bgcctestdir/ -shared -o $@ -O2 -fPIC -flto-partition=max -flto=2 $(srcdir)/ver_test_pr31830_lto.c -Wl,--version-script,$(srcdir)/ver_test_pr31830.script @GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed.stdout: weak_as_needed_a.so @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -dW --dyn-syms $< >$@ @GCC_TRUE@@NATIVE_LINKER_TRUE@weak_as_needed_a.so: gcctestdir/ld weak_as_needed_a.o weak_as_needed_b.so weak_as_needed_c.so diff --git a/gold/testsuite/ver_test_pr31830.script b/gold/testsuite/ver_test_pr31830.script new file mode 100644 index 0000000..0dcc47f --- /dev/null +++ b/gold/testsuite/ver_test_pr31830.script @@ -0,0 +1,6 @@ +GLIBC_2.2.5 { + global: + foo; + local: + *; +}; diff --git a/gold/testsuite/ver_test_pr31830.sh b/gold/testsuite/ver_test_pr31830.sh new file mode 100755 index 0000000..2a3c034 --- /dev/null +++ b/gold/testsuite/ver_test_pr31830.sh @@ -0,0 +1,61 @@ +#!/bin/sh + +# ver_test_pr31830.sh -- a test case for version scripts + +# Copyright (C) 2024 Free Software Foundation, Inc. + +# 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 test verifies that linker-generated symbols (e.g., _end) +# get correct version information even in the presence of +# a shared library that provides those symbols with different +# versions. + +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_missing() +{ + if grep -q "$2" "$1" + then + echo "Found unexpected symbol in $1:" + echo " $2" + echo "" + echo "Actual output below:" + cat "$1" + exit 1 + fi +} + +check ver_test_pr31830_a.syms "foo@GLIBC_2.2.5$" +check ver_test_pr31830_b.syms "foo@GLIBC_2.2.5$" + +check_missing ver_test_pr31830_a.syms "foo@@GLIBC_2.2.5$" +check_missing ver_test_pr31830_b.syms "foo@@GLIBC_2.2.5$" + +exit 0 diff --git a/gold/testsuite/ver_test_pr31830_a.c b/gold/testsuite/ver_test_pr31830_a.c new file mode 100644 index 0000000..bb57059 --- /dev/null +++ b/gold/testsuite/ver_test_pr31830_a.c @@ -0,0 +1,2 @@ +extern void foo(void); +void foo(void) {} diff --git a/gold/testsuite/ver_test_pr31830_b.c b/gold/testsuite/ver_test_pr31830_b.c new file mode 100644 index 0000000..aba07cc --- /dev/null +++ b/gold/testsuite/ver_test_pr31830_b.c @@ -0,0 +1,3 @@ +extern void __collector_foo_2_2(void); +__attribute__((__symver__("foo@GLIBC_2.2.5"))) +void __collector_foo_2_2(void) {} diff --git a/gold/testsuite/ver_test_pr31830_lto.c b/gold/testsuite/ver_test_pr31830_lto.c new file mode 100644 index 0000000..999dd63 --- /dev/null +++ b/gold/testsuite/ver_test_pr31830_lto.c @@ -0,0 +1,5 @@ +extern __inline __attribute__((__gnu_inline__)) void foo(void) {} +extern void __collector_foo_2_2(void); +__attribute__((__symver__("foo@GLIBC_2.2.5"))) +void __collector_foo_2_2(void) {} +void foo(void) {} diff --git a/gold/testsuite/ver_test_pr31830_lto.sh b/gold/testsuite/ver_test_pr31830_lto.sh new file mode 100755 index 0000000..4b939a1 --- /dev/null +++ b/gold/testsuite/ver_test_pr31830_lto.sh @@ -0,0 +1,61 @@ +#!/bin/sh + +# ver_test_pr31830_lto.sh -- a test case for version scripts with LTO + +# Copyright (C) 2024 Free Software Foundation, Inc. + +# 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 test verifies that linker-generated symbols (e.g., _end) +# get correct version information even in the presence of +# a shared library that provides those symbols with different +# versions. + +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_missing() +{ + if grep -q "$2" "$1" + then + echo "Found unexpected symbol in $1:" + echo " $2" + echo "" + echo "Actual output below:" + cat "$1" + exit 1 + fi +} + +check ver_test_pr31830_lto_a.syms "foo@GLIBC_2.2.5$" +check ver_test_pr31830_lto_b.syms "foo@GLIBC_2.2.5$" + +check_missing ver_test_pr31830_lto_a.syms "foo@@GLIBC_2.2.5$" +check_missing ver_test_pr31830_lto_b.syms "foo@@GLIBC_2.2.5$" + +exit 0 -- cgit v1.1