[PATCH] gold: Properly remove the versioned symbol

H.J. Lu hjl.tools@gmail.com
Sat Jun 1 05:09:26 GMT 2024


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): Change is_default_version
	to reference.  Don't give a multiple-definition error nor make
	the unversioned symbol as the default version if the hidden
	version from .symver is the same as the default version from the
	unversioned symbol.
	(Symbol_table::resolve<32, false>): Change is_default_versio to
	reference.
	(Symbol_table::resolve<32, true>): Likewise.
	(Symbol_table::resolve<64, false>): Likewise.
	(Symbol_table::resolve<64, true>): Likewise.
	* symtab.cc (Symbol_table::resolve): Updated.
	(Symbol_table::add_from_object): 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.
	* symtab.h (Symbol_table::resolve): Change is_default_version to
	reference.
	* 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_pr3183_a.c: Likewise.
	* testsuite/ver_test_pr3183_b.c: Likewise.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 gold/resolve.cc                        | 37 +++++++++++++---
 gold/symtab.cc                         | 20 ++++++---
 gold/symtab.h                          |  2 +-
 gold/testsuite/Makefile.am             | 11 +++++
 gold/testsuite/Makefile.in             | 18 ++++++++
 gold/testsuite/ver_test_pr31830.script |  6 +++
 gold/testsuite/ver_test_pr31830.sh     | 61 ++++++++++++++++++++++++++
 gold/testsuite/ver_test_pr3183_a.c     |  2 +
 gold/testsuite/ver_test_pr3183_b.c     |  3 ++
 9 files changed, 146 insertions(+), 14 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_pr3183_a.c
 create mode 100644 gold/testsuite/ver_test_pr3183_b.c

diff --git a/gold/resolve.cc b/gold/resolve.cc
index 777405dec1a..882f4f4fef6 100644
--- a/gold/resolve.cc
+++ b/gold/resolve.cc
@@ -245,7 +245,7 @@ Symbol_table::resolve(Sized_symbol<size>* to,
 		      unsigned int st_shndx, bool is_ordinary,
 		      unsigned int orig_st_shndx,
 		      Object* object, const char* version,
-		      bool is_default_version)
+		      bool &is_default_version)
 {
   bool to_is_ordinary;
   const unsigned int to_shndx = to->shndx(&to_is_ordinary);
@@ -256,13 +256,36 @@ Symbol_table::resolve(Sized_symbol<size>* to,
   // 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;
+    {
+      if (to->object() == object)
+	return;
+
+      // 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 ((version != NULL && version == to->version()))
+	{
+	  if (is_default_version && !to->is_default ())
+	    {
+	      // Don't make the unversioned symbol the default version.
+	      is_default_version = false;
+	      return;
+	    }
+
+	  if (!is_default_version && to->is_default ())
+	    {
+	      // Don't make the unversioned symbol the default version.
+	      to->set_is_not_default();
+	      is_default_version = false;
+	      return;
+	    }
+	}
+    }
 
   // Likewise for an absolute symbol defined twice with the same value.
   if (!is_ordinary
@@ -1147,7 +1170,7 @@ Symbol_table::resolve<32, false>(
     unsigned int orig_st_shndx,
     Object* object,
     const char* version,
-    bool is_default_version);
+    bool &is_default_version);
 
 template
 void
@@ -1159,7 +1182,7 @@ Symbol_table::resolve<32, true>(
     unsigned int orig_st_shndx,
     Object* object,
     const char* version,
-    bool is_default_version);
+    bool &is_default_version);
 #endif
 
 #if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
@@ -1173,7 +1196,7 @@ Symbol_table::resolve<64, false>(
     unsigned int orig_st_shndx,
     Object* object,
     const char* version,
-    bool is_default_version);
+    bool &is_default_version);
 
 template
 void
@@ -1185,7 +1208,7 @@ Symbol_table::resolve<64, true>(
     unsigned int orig_st_shndx,
     Object* object,
     const char* version,
-    bool is_default_version);
+    bool &is_default_version);
 #endif
 
 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 9a55e6ea511..8422b208999 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -740,8 +740,9 @@ Symbol_table::resolve(Sized_symbol<size>* to, const Sized_symbol<size>* from)
   esym.put_st_other(from->visibility(), from->nonvis());
   bool is_ordinary;
   unsigned int shndx = from->shndx(&is_ordinary);
+  bool is_default_version = true;
   this->resolve(to, esym.sym(), shndx, is_ordinary, shndx, from->object(),
-		from->version(), true);
+		from->version(), is_default_version);
   if (from->in_reg())
     to->set_in_reg();
   if (from->in_dyn())
@@ -1002,6 +1003,7 @@ Symbol_table::add_from_object(Object* object,
       // Commons from plugins are just placeholders.
       was_common = ret->is_common() && ret->object()->pluginobj() == NULL;
 
+      bool orig_is_default_version = is_default_version;
       this->resolve(ret, sym, st_shndx, is_ordinary, orig_st_shndx, object,
 		    version, is_default_version);
       if (parameters->options().gc_sections())
@@ -1015,10 +1017,8 @@ Symbol_table::add_from_object(Object* object,
 	  bool dummy;
 	  if (version != NULL
 	      && ret->source() == Symbol::FROM_OBJECT
-	      && ret->object() == object
 	      && is_ordinary
-	      && ret->shndx(&dummy) == st_shndx
-	      && ret->is_default())
+	      && ret->shndx(&dummy) == st_shndx)
 	    {
 	      // We have seen NAME/VERSION already, and marked it as the
 	      // default version, but now we see a definition for
@@ -1032,9 +1032,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 (orig_is_default_version)
+		this->table_.erase(std::make_pair(name_key, vnull_key));
+	      else if (ret->object() == object && ret->is_default())
+		{
+		  ret->set_is_not_default();
+		  this->table_.erase(std::make_pair(name_key, vnull_key));
+		}
 	    }
 	}
     }
diff --git a/gold/symtab.h b/gold/symtab.h
index 0a1f6a63a76..c7792678b1d 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1786,7 +1786,7 @@ class Symbol_table
 	  unsigned int st_shndx, bool is_ordinary,
 	  unsigned int orig_st_shndx,
 	  Object*, const char* version,
-	  bool is_default_version);
+	  bool &is_default_version);
 
   template<int size, bool big_endian>
   void
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 6e9af67b22d..5797874e98d 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -2040,6 +2040,17 @@ 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_pr3183_a.o ver_test_pr3183_b.o $(srcdir)/ver_test_pr31830.script
+	gcctestdir/ld -shared -o $@ ver_test_pr3183_a.o ver_test_pr3183_b.o --version-script $(srcdir)/ver_test_pr31830.script
+ver_test_pr31830_b.so: gcctestdir/ld ver_test_pr3183_a.o ver_test_pr3183_b.o $(srcdir)/ver_test_pr31830.script
+	gcctestdir/ld -shared -o $@ ver_test_pr3183_b.o ver_test_pr3183_a.o --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..4e0bc83e7eb 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -487,6 +487,7 @@ 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@	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 +545,8 @@ 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@	weak_as_needed.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	protected_3.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	relro_test.stdout \
@@ -5978,6 +5981,13 @@ 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)
 weak_as_needed.sh.log: weak_as_needed.sh
 	@p='weak_as_needed.sh'; \
 	b='weak_as_needed.sh'; \
@@ -9031,6 +9041,14 @@ 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_pr3183_a.o ver_test_pr3183_b.o $(srcdir)/ver_test_pr31830.script
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	gcctestdir/ld -shared -o $@ ver_test_pr3183_a.o ver_test_pr3183_b.o --version-script $(srcdir)/ver_test_pr31830.script
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_pr31830_b.so: gcctestdir/ld ver_test_pr3183_a.o ver_test_pr3183_b.o $(srcdir)/ver_test_pr31830.script
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	gcctestdir/ld -shared -o $@ ver_test_pr3183_b.o ver_test_pr3183_a.o --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_pr3183_a.c b/gold/testsuite/ver_test_pr3183_a.c
new file mode 100644
index 00000000000..bb57059bf7e
--- /dev/null
+++ b/gold/testsuite/ver_test_pr3183_a.c
@@ -0,0 +1,2 @@
+extern void foo(void);
+void foo(void) {}
diff --git a/gold/testsuite/ver_test_pr3183_b.c b/gold/testsuite/ver_test_pr3183_b.c
new file mode 100644
index 00000000000..aba07cc6305
--- /dev/null
+++ b/gold/testsuite/ver_test_pr3183_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) {}
-- 
2.45.1



More information about the Binutils mailing list