[PATCH v5] gold: Properly remove the versioned symbol

Fangrui Song i@maskray.me
Tue Jun 4 19:22:46 GMT 2024


On Mon, Jun 3, 2024 at 5:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> If there are no objections, I will check in the v5 patch next week.
>
> Thanks.
>
> H.J.
> On Sun, Jun 2, 2024, 4:29 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> 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 shouldn't provide the default
>> version for foo nor 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 <hjl.tools@gmail.com>
>> ---
>>  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
>>
>> diff --git a/gold/resolve.cc b/gold/resolve.cc
>> index 777405dec1a..f05589551f4 100644
>> --- a/gold/resolve.cc
>> +++ b/gold/resolve.cc
>> @@ -250,20 +250,6 @@ Symbol_table::resolve(Sized_symbol<size>* 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 9a55e6ea511..5857dd7b098 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<size>(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 6e9af67b22d..a40f7624395 100644
>> --- a/gold/testsuite/Makefile.am
>> +++ b/gold/testsuite/Makefile.am
>> @@ -2040,6 +2040,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 db299dd97f6..78ca73acb17 100644
>> --- a/gold/testsuite/Makefile.in
>> +++ b/gold/testsuite/Makefile.in
>> @@ -487,6 +487,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 \
>> @@ -544,6 +546,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 \
>> @@ -5978,6 +5984,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'; \
>> @@ -9031,6 +9051,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 00000000000..0dcc47f4f5c
>> --- /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 00000000000..2a3c0347461
>> --- /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 00000000000..bb57059bf7e
>> --- /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 00000000000..aba07cc6305
>> --- /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 00000000000..999dd63f884
>> --- /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 00000000000..4b939a161ac
>> --- /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
>> --
>> 2.45.1
>>
>>

Both GNU ld and not-too-ancient lld have the following behavior:

When defined foo@GLIBC_2.2.5 from x.o and defined foo from y.o are
combined, and a version script assigns "GLIBC_2.2.5" to foo,
foo@GLIBC_2.2.5 is retained and foo is dropped. There is no duplicate
definition error.

ld.bfd --version-script=x.t -shared -o x.so x.o y.o  # ok
gold --version-script=x.t -shared -o x.so x.o y.o  # ok

This patch will make gold do so. The description might be reworded to
make this clearer.

---

However, I wonder whether gold's current behavior (reporting a
duplicate definition error) is better.
x.o and y.o provide different definitions in this case.


More information about the Binutils mailing list