This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

gold patch committed: handle empty version tag


PR 6647 is about a version script which omits the version tag.  This
is useful if all you want to do is force some symbols in a shared
library to be local, and you don't want to mark up the source code
with annotations about symbol visibility.  Unfortunately, empty
version tags crashed gold.

I committed this patch to fix the problem, along with a test case.

Ian


2008-07-23  Ian Lance Taylor  <iant@google.com>

	PR 6647
	* script.cc (Version_script_info::get_versions): Don't add empty
	version tag to return value.
	(Version_script_info::get_symbol_version_helper): Change return
	type to bool.  Add pversion parameter.  Change all callers.
	(script_register_vers_node): Don't require a non-NULL tag.
	* script.h (class Version_script_info): Update declarations.
	(Version_script_info::get_symbol_version): Change return type to
	bool.  Add version parameter.  Change all callers.
	* symtab.cc (Sized_symbol::add_from_relobj): Rework version
	handling.  Handle an empty version from a version script.
	(Symbol_table::define_special_symbol): Likewise.
	* testsuite/ver_test_10.script: New file.
	* testsuite/ver_test_10.sh: New file.
	* testsuite/Makefile.am (check_SCRIPTS): Add ver_test_10.sh.
	(check_DATA): Add ver_test_10.syms.
	(ver_test_10.syms, ver_test_10.so): New target.
	* testsuite/Makefile.in: Rebuild.


Index: script.cc
===================================================================
RCS file: /cvs/src/src/gold/script.cc,v
retrieving revision 1.44
diff -p -u -r1.44 script.cc
--- script.cc	30 Jun 2008 20:59:54 -0000	1.44
+++ script.cc	23 Jul 2008 23:37:52 -0000
@@ -1732,7 +1732,8 @@ Version_script_info::get_versions() cons
 {
   std::vector<std::string> ret;
   for (size_t j = 0; j < version_trees_.size(); ++j)
-    ret.push_back(version_trees_[j]->tag);
+    if (!this->version_trees_[j]->tag.empty())
+      ret.push_back(this->version_trees_[j]->tag);
   return ret;
 }
 
@@ -1753,9 +1754,16 @@ Version_script_info::get_dependencies(co
   return ret;
 }
 
-const std::string&
+// Look up SYMBOL_NAME in the list of versions.  If CHECK_GLOBAL is
+// true look at the globally visible symbols, otherwise look at the
+// symbols listed as "local:".  Return true if the symbol is found,
+// false otherwise.  If the symbol is found, then if PVERSION is not
+// NULL, set *PVERSION to the version.
+
+bool
 Version_script_info::get_symbol_version_helper(const char* symbol_name,
-                                               bool check_global) const
+                                               bool check_global,
+					       std::string* pversion) const
 {
   for (size_t j = 0; j < version_trees_.size(); ++j)
     {
@@ -1796,11 +1804,14 @@ Version_script_info::get_symbol_version_
             if (demangled_name != NULL)
               free(demangled_name);
             if (matched)
-              return version_trees_[j]->tag;
+	      {
+		if (pversion != NULL)
+		  *pversion = this->version_trees_[j]->tag;
+		return true;
+	      }
           }
     }
-  static const std::string empty = "";
-  return empty;
+  return false;
 }
 
 struct Version_dependency_list*
@@ -2207,9 +2218,9 @@ script_register_vers_node(void*,
 			  struct Version_dependency_list *deps)
 {
   gold_assert(tree != NULL);
-  gold_assert(tag != NULL);
   tree->dependencies = deps;
-  tree->tag = std::string(tag, taglen);
+  if (tag != NULL)
+    tree->tag = std::string(tag, taglen);
 }
 
 // Add a dependencies to the list of existing dependencies, if any,
Index: script.h
===================================================================
RCS file: /cvs/src/src/gold/script.h,v
retrieving revision 1.19
diff -p -u -r1.19 script.h
--- script.h	21 May 2008 21:37:44 -0000	1.19
+++ script.h	23 Jul 2008 23:37:52 -0000
@@ -138,22 +138,17 @@ class Version_script_info
   empty() const
   { return this->version_trees_.empty(); }
 
-  // Return the version associated with the given symbol name.
-  // Strings are allocated out of the stringpool given in the
-  // constructor.  Strings are allocated out of the stringpool given
-  // in the constructor.
-  const std::string&
-  get_symbol_version(const char* symbol) const
-  { return get_symbol_version_helper(symbol, true); }
+  // If there is a version associated with SYMBOL, return true, and
+  // set *VERSION to the version.  Otherwise, return false.
+  bool
+  get_symbol_version(const char* symbol, std::string* version) const
+  { return this->get_symbol_version_helper(symbol, true, version); }
 
-  // Return whether this symbol matches the local: section of a
-  // version script (it doesn't matter which).
+  // Return whether this symbol matches the local: section of some
+  // version.
   bool
   symbol_is_local(const char* symbol) const
-  {
-    return (get_symbol_version(symbol).empty()
-            && !get_symbol_version_helper(symbol, false).empty());
-  }
+  { return this->get_symbol_version_helper(symbol, false, NULL); }
 
   // Return the names of versions defined in the version script.
   // Strings are allocated out of the stringpool given in the
@@ -186,8 +181,9 @@ class Version_script_info
   void
   print_expression_list(FILE* f, const Version_expression_list*) const;
 
-  const std::string& get_symbol_version_helper(const char* symbol,
-                                               bool check_global) const;
+  bool get_symbol_version_helper(const char* symbol,
+				 bool check_global,
+				 std::string* pversion) const;
 
   std::vector<struct Version_dependency_list*> dependency_lists_;
   std::vector<struct Version_expression_list*> expression_lists_;
Index: symtab.cc
===================================================================
RCS file: /cvs/src/src/gold/symtab.cc,v
retrieving revision 1.106
diff -p -u -r1.106 symtab.cc
--- symtab.cc	23 Jul 2008 16:19:59 -0000	1.106
+++ symtab.cc	23 Jul 2008 23:37:52 -0000
@@ -885,6 +885,7 @@ Symbol_table::add_from_relobj(
       // name from the version name.  If there are two '@' characters,
       // this is the default version.
       const char* ver = strchr(name, '@');
+      Stringpool::Key ver_key = 0;
       int namelen = 0;
       // DEF: is the version default?  LOCAL: is the symbol forced local?
       bool def = false;
@@ -900,26 +901,37 @@ Symbol_table::add_from_relobj(
 	      def = true;
 	      ++ver;
 	    }
+	  ver = this->namepool_.add(ver, true, &ver_key);
         }
       // We don't want to assign a version to an undefined symbol,
       // even if it is listed in the version script.  FIXME: What
       // about a common symbol?
-      else if (!version_script_.empty()
-	       && st_shndx != elfcpp::SHN_UNDEF)
-        {
-          // The symbol name did not have a version, but
-          // the version script may assign a version anyway.
-          namelen = strlen(name);
-          def = true;
-          // Check the global: entries from the version script.
-          const std::string& version =
-              version_script_.get_symbol_version(name);
-          if (!version.empty())
-            ver = version.c_str();
-          // Check the local: entries from the version script
-          if (version_script_.symbol_is_local(name))
-            local = true;
-        }
+      else
+	{
+	  namelen = strlen(name);
+	  if (!this->version_script_.empty()
+	      && st_shndx != elfcpp::SHN_UNDEF)
+	    {
+	      // The symbol name did not have a version, but the
+	      // version script may assign a version anyway.
+	      std::string version;
+	      if (this->version_script_.get_symbol_version(name, &version))
+		{
+		  // The version can be empty if the version script is
+		  // only used to force some symbols to be local.
+		  if (!version.empty())
+		    {
+		      ver = this->namepool_.add_with_length(version.c_str(),
+							    version.length(),
+							    true,
+							    &ver_key);
+		      def = true;
+		    }
+		}
+	      else if (this->version_script_.symbol_is_local(name))
+		local = true;
+	    }
+	}
 
       elfcpp::Sym<size, big_endian>* psym = &sym;
       unsigned char symbuf[sym_size];
@@ -944,29 +956,17 @@ Symbol_table::add_from_relobj(
 	  psym = &sym2;
 	}
 
+      Stringpool::Key name_key;
+      name = this->namepool_.add_with_length(name, namelen, true,
+					     &name_key);
+
       Sized_symbol<size>* res;
-      if (ver == NULL)
-	{
-	  Stringpool::Key name_key;
-	  name = this->namepool_.add(name, true, &name_key);
-	  res = this->add_from_object(relobj, name, name_key, NULL, 0,
-				      false, *psym, st_shndx, is_ordinary,
-				      orig_st_shndx);
-          if (local)
-	    this->force_local(res);
-	}
-      else
-	{
-	  Stringpool::Key name_key;
-	  name = this->namepool_.add_with_length(name, namelen, true,
-						 &name_key);
-	  Stringpool::Key ver_key;
-	  ver = this->namepool_.add(ver, true, &ver_key);
+      res = this->add_from_object(relobj, name, name_key, ver, ver_key,
+				  def, *psym, st_shndx, is_ordinary,
+				  orig_st_shndx);
 
-	  res = this->add_from_object(relobj, name, name_key, ver, ver_key,
-				      def, *psym, st_shndx, is_ordinary,
-				      orig_st_shndx);
-	}
+      if (local)
+	this->force_local(res);
 
       (*sympointers)[i] = res;
     }
@@ -1270,11 +1270,14 @@ Symbol_table::define_special_symbol(cons
 
   // If the caller didn't give us a version, see if we get one from
   // the version script.
+  std::string v;
   if (*pversion == NULL)
     {
-      const std::string& v(this->version_script_.get_symbol_version(*pname));
-      if (!v.empty())
-	*pversion = v.c_str();
+      if (this->version_script_.get_symbol_version(*pname, &v))
+	{
+	  if (!v.empty())
+	    *pversion = v.c_str();
+	}
     }
 
   if (only_if_ref)
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.72
diff -p -u -r1.72 Makefile.am
--- testsuite/Makefile.am	23 Jul 2008 14:36:09 -0000	1.72
+++ testsuite/Makefile.am	23 Jul 2008 23:37:52 -0000
@@ -784,6 +784,13 @@ ver_test_9.so: ver_test_9.o ver_test_4.s
 ver_test_9.o: ver_test_9.cc
 	$(CXXCOMPILE) -c -fpic -o $@ $<
 
+check_SCRIPTS += ver_test_10.sh
+check_DATA += ver_test_10.syms
+ver_test_10.syms: ver_test_10.so
+	$(TEST_READELF) -s $< >$@ 2>/dev/null
+ver_test_10.so: gcctestdir/ld ver_test_2.o ver_test_10.script
+	$(CXXLINK) -Bgcctestdir/ -shared -Wl,--version-script,$(srcdir)/ver_test_10.script ver_test_2.o
+
 check_PROGRAMS += protected_1
 protected_1_SOURCES = \
 	protected_main_1.cc protected_main_2.cc protected_main_3.cc
Index: testsuite/ver_test_10.script
===================================================================
RCS file: testsuite/ver_test_10.script
diff -N testsuite/ver_test_10.script
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/ver_test_10.script	23 Jul 2008 23:37:52 -0000
@@ -0,0 +1,30 @@
+## ver_test_10.script -- a test case for gold
+
+## Copyright 2008 Free Software Foundation, Inc.
+## Written by Ian Lance Taylor <iant@google.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.
+
+# Test having a version script with no version.
+
+{
+  global:
+    t3_2;
+  local:
+    *;
+};
Index: testsuite/ver_test_10.sh
===================================================================
RCS file: testsuite/ver_test_10.sh
diff -N testsuite/ver_test_10.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/ver_test_10.sh	23 Jul 2008 23:37:52 -0000
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+# ver_test_10.sh -- test global/local symbols
+
+# Copyright 2008 Free Software Foundation, Inc.
+# Written by Ian Lance Taylor <iant@google.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.
+
+# This file goes with ver_test_4.script and ver_test_5.script.  The
+# symbol t2_2 is not defined when ver_test_5.script is used.
+
+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_10.syms "GLOBAL.*t3_2"
+check ver_test_10.syms "LOCAL.*t4_2"
+
+exit 0

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]