Fix symbol versioning problems in PR 18703.

If a symbol is defined with ".symver foo,foo@VER", the assembler
creates two symbols in the object: one unversioned, and one with
the (non-default) version "VER". If foo is listed in a version
script, gold would then make the first of those symbols the
default version, and would ignore the second symbol as a
duplicate, without making it a non-default version. While this is
arguably reasonable behavior, it doesn't match Gnu ld behavior,
so this patch fixes that by allowing the second definition to
override the first by resetting the "default version" indication.

Several test cases from the Gnu ld testsuite also exposed another
related problem, where a symbol defined with ".symver foo,foo@",
placed into a shared library, is not handled properly by gold.
This patch also fixes that case, binding the symbol to the base
version.

gold/
	PR gold/18703
	* dynobj.cc (Versions::record_version): Handle symbol defined with
	base version.
	(Versions::symbol_section_contents): Likewise.
	* symtab.h (Symbol::set_is_not_default): New class method.
	(Symbol_table::resolve): Add is_default_version parameter.
	(Symbol_table::should_override): Likewise.
	* resolve.cc (Symbol_table::resolve): Add is_default_version parameter,
	and pass to should_override. Adjust all callers and explicit
	instantiations.
	(Symbol_table::should_override): Add is_default_value parameter;
	allow default version in a dynamic object to override existing
	definition from same object.
	* symtab.cc (Symbol_table::add_from_object): Handle case where same
	symbol is defined as unversioned and non-default version in the same
	object.
	* testsuite/Makefile.am (ver_test_13): New test case.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/ver_test_4.cc: Add test for symbol with base version.
	* testsuite/ver_test_4.sh: Likewise.
	* testsuite/ver_test_13.c: New source file.
	* testsuite/ver_test_13.script: New version script.
	* testsuite/ver_test_13.sh: New test case.

diff --git a/gold/dynobj.cc b/gold/dynobj.cc
index 13e3f616a606979ddcb13243122a53eae44a5a7f..9ab6bf885919a846b077395443c42dd17ffa8202 100644 (file)
--- a/gold/dynobj.cc
+++ b/gold/dynobj.cc
@@ -1499,6 +1499,10 @@ Versions::record_version(const Symbol_table* symtab,
   gold_assert(!this->is_finalized_);
   gold_assert(sym->version() != NULL);
 
+  // A symbol defined as "sym@" is bound to an unspecified base version.
+  if (sym->version()[0] == '\0')
+    return;
+
   Stringpool::Key version_key;
   const char* version = dynpool->add(sym->version(), false, &version_key);
 
@@ -1731,15 +1735,17 @@ Versions::symbol_section_contents(const Symbol_table* symtab,
     {
       unsigned int version_index;
       const char* version = (*p)->version();
-      if (version != NULL)
-       version_index = this->version_index(symtab, dynpool, *p);
-      else
+      if (version == NULL)
        {
          if ((*p)->is_defined() && !(*p)->is_from_dynobj())
            version_index = elfcpp::VER_NDX_GLOBAL;
          else
            version_index = elfcpp::VER_NDX_LOCAL;
        }
+      else if (version[0] == '\0')
+        version_index = elfcpp::VER_NDX_GLOBAL;
+      else
+       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())

diff --git a/gold/resolve.cc b/gold/resolve.cc
index 22d1e788261e2dfb9379200ad05798a702226d3c..2dcf7b5b1e8c6431029fe3f53f4b43d41d841bc2 100644 (file)
--- a/gold/resolve.cc
+++ b/gold/resolve.cc
@@ -243,7 +243,8 @@ Symbol_table::resolve(Sized_symbol<size>* to,
                      const elfcpp::Sym<size, big_endian>& sym,
                      unsigned int st_shndx, bool is_ordinary,
                      unsigned int orig_st_shndx,
-                     Object* object, const char* version)
+                     Object* object, const char* version,
+                     bool 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
@@ -380,7 +381,7 @@ Symbol_table::resolve(Sized_symbol<size>* to,
   typename Sized_symbol<size>::Size_type tosize = to->symsize();
   if (Symbol_table::should_override(to, frombits, fromtype, OBJECT,
                                    object, &adjust_common_sizes,
-                                   &adjust_dyndef))
+                                   &adjust_dyndef, is_default_version))
     {
       elfcpp::STB tobinding = to->binding();
       typename Sized_symbol<size>::Value_type tovalue = to->value();
@@ -449,7 +450,7 @@ bool
 Symbol_table::should_override(const Symbol* to, unsigned int frombits,
                              elfcpp::STT fromtype, Defined defined,
                              Object* object, bool* adjust_common_sizes,
-                             bool* adjust_dyndef)
+                             bool* adjust_dyndef, bool is_default_version)
 {
   *adjust_common_sizes = false;
   *adjust_dyndef = false;
@@ -596,9 +597,19 @@ Symbol_table::should_override(const Symbol* to, unsigned int frombits,
 
     case DEF * 16 + DYN_DEF:
     case WEAK_DEF * 16 + DYN_DEF:
+      // Ignore a dynamic definition if we already have a definition.
+      return false;
+
     case DYN_DEF * 16 + DYN_DEF:
     case DYN_WEAK_DEF * 16 + DYN_DEF:
-      // Ignore a dynamic definition if we already have a definition.
+      // Ignore a dynamic definition if we already have a definition,
+      // unless the existing definition is an unversioned definition
+      // in the same dynamic object, and the new definition is a
+      // default version.
+      if (to->object() == object
+          && to->version() == NULL
+          && is_default_version)
+        return true;
       return false;
 
     case UNDEF * 16 + DYN_DEF:
@@ -919,7 +930,7 @@ Symbol_table::should_override_with_special(const Symbol* to,
   unsigned int frombits = global_flag | regular_flag | def_flag;
   bool ret = Symbol_table::should_override(to, frombits, fromtype, defined,
                                           NULL, &adjust_common_sizes,
-                                          &adjust_dyn_def);
+                                          &adjust_dyn_def, false);
   gold_assert(!adjust_common_sizes && !adjust_dyn_def);
   return ret;
 }
@@ -1051,7 +1062,8 @@ Symbol_table::resolve<32, false>(
     bool is_ordinary,
     unsigned int orig_st_shndx,
     Object* object,
-    const char* version);
+    const char* version,
+    bool is_default_version);
 
 template
 void
@@ -1062,7 +1074,8 @@ Symbol_table::resolve<32, true>(
     bool is_ordinary,
     unsigned int orig_st_shndx,
     Object* object,
-    const char* version);
+    const char* version,
+    bool is_default_version);
 #endif
 
 #if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
@@ -1075,7 +1088,8 @@ Symbol_table::resolve<64, false>(
     bool is_ordinary,
     unsigned int orig_st_shndx,
     Object* object,
-    const char* version);
+    const char* version,
+    bool is_default_version);
 
 template
 void
@@ -1086,7 +1100,8 @@ Symbol_table::resolve<64, true>(
     bool is_ordinary,
     unsigned int orig_st_shndx,
     Object* object,
-    const char* version);
+    const char* 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 31ecc5c2638056fed5565b349aace636333de25e..7e30fbf74661f90427e4903ca4de56e1f4a0508b 100644 (file)
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -737,7 +737,7 @@ Symbol_table::resolve(Sized_symbol<size>* to, const Sized_symbol<size>* from)
   bool is_ordinary;
   unsigned int shndx = from->shndx(&is_ordinary);
   this->resolve(to, esym.sym(), shndx, is_ordinary, shndx, from->object(),
-               from->version());
+               from->version(), true);
   if (from->in_reg())
     to->set_in_reg();
   if (from->in_dyn())
@@ -991,13 +991,38 @@ Symbol_table::add_from_object(Object* object,
       was_common = ret->is_common() && ret->object()->pluginobj() == NULL;
 
       this->resolve(ret, sym, st_shndx, is_ordinary, orig_st_shndx, object,
-                   version);
+                   version, is_default_version);
       if (parameters->options().gc_sections())
         this->gc_mark_dyn_syms(ret);
 
       if (is_default_version)
        this->define_default_version<size, big_endian>(ret, insdefault.second,
                                                       insdefault.first);
+      else if (version != NULL && ret->is_default())
+       {
+         // We have seen NAME/VERSION already, and marked it as the
+         // default version, but now we see a definition for
+         // NAME/VERSION that is not the default version. This can
+         // happen when the assembler generates two symbols for
+         // a symbol as a result of a ".symver foo,foo@VER"
+         // directive. We see the first unversioned symbol and
+         // we may mark it as the default version (from a
+         // version script); then we see the second versioned
+         // symbol and we need to override the first.
+         // In any other case, the two symbols should have generated
+         // a multiple definition error.
+         // (See PR gold/18703.)
+         bool dummy;
+         if (ret->source() == Symbol::FROM_OBJECT
+             && ret->object() == object
+             && is_ordinary
+             && ret->shndx(&dummy) == st_shndx)
+           {
+             ret->set_is_not_default();
+             const Stringpool::Key vnull_key = 0;
+             this->table_.erase(std::make_pair(name_key, vnull_key));
+           }
+       }
     }
   else
     {
@@ -1015,7 +1040,7 @@ Symbol_table::add_from_object(Object* object,
          was_common = ret->is_common() && ret->object()->pluginobj() == NULL;
 
          this->resolve(ret, sym, st_shndx, is_ordinary, orig_st_shndx, object,
-                       version);
+                       version, is_default_version);
           if (parameters->options().gc_sections())
             this->gc_mark_dyn_syms(ret);
          ins.first->second = ret;

diff --git a/gold/symtab.h b/gold/symtab.h
index 2c9aa32eb0a4a245b684770c254918d6382a75e9..1a1f1c78633b76d05224f5d6ec6878b931a8d8a7 100644 (file)
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -140,6 +140,11 @@ class Symbol
   set_is_default()
   { this->is_def_ = true; }
 
+  // Set that this version is not the default for this symbol name.
+  void
+  set_is_not_default()
+  { this->is_def_ = false; }
+
   // Return the symbol's name as name@version (or name@@version).
   std::string
   versioned_name() const;
@@ -1706,7 +1711,8 @@ class Symbol_table
          const elfcpp::Sym<size, big_endian>& sym,
          unsigned int st_shndx, bool is_ordinary,
          unsigned int orig_st_shndx,
-         Object*, const char* version);
+         Object*, const char* version,
+         bool is_default_version);
 
   template<int size, bool big_endian>
   void
@@ -1725,7 +1731,7 @@ class Symbol_table
   // resolve.cc.
   static bool
   should_override(const Symbol*, unsigned int, elfcpp::STT, Defined,
-                 Object*, bool*, bool*);
+                 Object*, bool*, bool*, bool);
 
   // Report a problem in symbol resolution.
   static void

diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index ca070163a720e5f7691ec7d1b6f7cab808c86746..5ceaeef31780e3f63bde482c2e329557abd3d4b9 100644 (file)
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1563,6 +1563,15 @@ ver_test_12_LDADD = ver_test_12.o
 ver_test_12.o: gcctestdir/ld ver_test_1.o ver_test_2.o ver_test_4.o
        gcctestdir/ld -r -o $@ ver_test_1.o ver_test_2.o ver_test_4.o
 
+check_SCRIPTS += ver_test_13.sh
+check_DATA += ver_test_13.syms
+ver_test_13.syms: ver_test_13.so
+       $(TEST_READELF) -s $< >$@ 2>/dev/null
+ver_test_13.so: gcctestdir/ld ver_test_13.o ver_test_13.script
+       $(LINK) -Bgcctestdir/ -shared -Wl,--version-script,$(srcdir)/ver_test_13.script ver_test_13.o
+ver_test_13.o: ver_test_13.c
+       $(COMPILE) -c -fpic -o $@ $<
+
 check_PROGRAMS += protected_1
 protected_1_SOURCES = \
        protected_main_1.cc protected_main_2.cc protected_main_3.cc

diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index d4cc5dca69ea1c9cd629b399e60bfe1aa923e2ee..9cabcd81c438d385f302a2a50685646f2bff8c2b 100644 (file)
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -327,7 +327,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_1.sh ver_test_2.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_4.sh ver_test_5.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_7.sh ver_test_10.sh \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@ relro_test.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_13.sh relro_test.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_matching_test.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_3.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_4.sh \
@@ -371,7 +371,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_1.syms ver_test_2.syms \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_4.syms ver_test_5.syms \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_7.syms ver_test_10.syms \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@ protected_3.err \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_test_13.syms protected_3.err \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ relro_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ ver_matching_test.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ script_test_3.stdout \
@@ -4432,6 +4432,8 @@ ver_test_7.sh.log: ver_test_7.sh
        @p='ver_test_7.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 ver_test_10.sh.log: ver_test_10.sh
        @p='ver_test_10.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+ver_test_13.sh.log: ver_test_13.sh
+       @p='ver_test_13.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 relro_test.sh.log: relro_test.sh
        @p='relro_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 ver_matching_test.sh.log: ver_matching_test.sh
@@ -5747,6 +5749,12 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_AR) rc $@ $^
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_12.o: gcctestdir/ld ver_test_1.o ver_test_2.o ver_test_4.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ gcctestdir/ld -r -o $@ ver_test_1.o ver_test_2.o ver_test_4.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_13.syms: ver_test_13.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -s $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_13.so: gcctestdir/ld ver_test_13.o ver_test_13.script
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(LINK) -Bgcctestdir/ -shared -Wl,--version-script,$(srcdir)/ver_test_13.script ver_test_13.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_13.o: ver_test_13.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(COMPILE) -c -fpic -o $@ $<
 
 @GCC_TRUE@@NATIVE_LINKER_TRUE@protected_1.so: gcctestdir/ld protected_1_pic.o protected_2_pic.o protected_3_pic.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Bgcctestdir/ -shared protected_1_pic.o protected_2_pic.o protected_3_pic.o

diff --git a/gold/testsuite/ver_test_13.c b/gold/testsuite/ver_test_13.c
new file mode 100644 (file)
index 0000000..01f4ba7
--- /dev/null
+++ b/gold/testsuite/ver_test_13.c
@@ -0,0 +1,7 @@
+__asm__ (".symver foo, foo@VER_0");
+
+int foo(void);
+
+int foo(void) {
+  return 0;
+}

diff --git a/gold/testsuite/ver_test_13.script b/gold/testsuite/ver_test_13.script
new file mode 100644 (file)
index 0000000..f0205cf
--- /dev/null
+++ b/gold/testsuite/ver_test_13.script
@@ -0,0 +1,4 @@
+VER_0 {
+  global:
+       foo;
+};

diff --git a/gold/testsuite/ver_test_13.sh b/gold/testsuite/ver_test_13.sh
new file mode 100755 (executable)
index 0000000..b0435ff
--- /dev/null
+++ b/gold/testsuite/ver_test_13.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+# ver_test_13.sh -- a test case for version script matching
+
+# Copyright (C) 2015 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@gmail.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 test verifies that a symbol declared with .symver as a
+# non-default version does not get overridden by the version
+# script and declared as a default version.
+# (See PR gold/18703.)
+
+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_13.syms "foo@VER_0$"
+check_missing ver_test_13.syms "foo@@VER_0"
+
+exit 0

diff --git a/gold/testsuite/ver_test_4.cc b/gold/testsuite/ver_test_4.cc
index 7a5544e796cb403db6ca8a59bf1865ee4b1528b3..2fdba21ae957f256878d35a4d09c0ae32d30d139 100644 (file)
--- a/gold/testsuite/ver_test_4.cc
+++ b/gold/testsuite/ver_test_4.cc
@@ -22,6 +22,16 @@
 
 #include "ver_test.h"
 
+__asm__(".symver t1_2_orig,t1_2@");
+
+extern "C"
+int
+t1_2_orig()
+{
+  TRACE
+  return 12;
+}
+
 __asm__(".symver t1_2_a,t1_2@@VER2");
 
 extern "C"

diff --git a/gold/testsuite/ver_test_4.sh b/gold/testsuite/ver_test_4.sh
index 05305b1ff065d0d4e975760e9b45700422c40bad..bc67ef97769072bff8d1160d77c21a1789d81394 100755 (executable)
--- a/gold/testsuite/ver_test_4.sh
+++ b/gold/testsuite/ver_test_4.sh
@@ -24,17 +24,18 @@
 
 check()
 {
-    if ! grep -q "$2" "$1"
+    if ! sed '/\.symtab/q' "$1" | grep -q "$2"
     then
        echo "Did not find expected symbol in $1:"
        echo "   $2"
        echo ""
        echo "Actual output below:"
-       cat "$1"
+       sed '/\.symtab/q' "$1"
        exit 1
     fi
 }
 
+check ver_test_4.syms "t1_2\$"
 check ver_test_4.syms "t1_2@@VER2"
 check ver_test_4.syms "t2_2@VER1"
 check ver_test_4.syms "t2_2@@VER2"