Fix internal error caused by conflicting default version definitions.
authorCary Coutant <ccoutant@gmail.com>
Mon, 23 Apr 2018 16:27:35 +0000 (09:27 -0700)
committerCary Coutant <ccoutant@gmail.com>
Tue, 24 Apr 2018 20:51:24 +0000 (13:51 -0700)
gold/
PR gold/16504
* dynobj.cc (Versions::symbol_section_contents): Don't set
VERSYM_HIDDEN flag for undefined symbols.
* symtab.cc (Symbol_table::add_from_object): Don't override default
version definition with a different default version.
* symtab.h (Symbol::from_dyn): New method.
* testsuite/plugin_test.c (struct sym_info): Add ver field.
(claim_file_hook): Pass symbol version to plugin API.
(parse_readelf_line): Parse symbol version.
* testsuite/Makefile.am (ver_test_pr16504): New test case.
* testsuite/Makefile.in: Regenerate.
* testsuite/ver_test_pr16504.sh: New test script.
* testsuite/ver_test_pr16504_a.c: New source file.
* testsuite/ver_test_pr16504_a.script: New version script.
* testsuite/ver_test_pr16504_b.c: New source file.
* testsuite/ver_test_pr16504_b.script: New version script.

12 files changed:
gold/ChangeLog
gold/dynobj.cc
gold/symtab.cc
gold/symtab.h
gold/testsuite/Makefile.am
gold/testsuite/Makefile.in
gold/testsuite/plugin_test.c
gold/testsuite/ver_test_pr16504.sh [new file with mode: 0755]
gold/testsuite/ver_test_pr16504_a.c [new file with mode: 0644]
gold/testsuite/ver_test_pr16504_a.script [new file with mode: 0644]
gold/testsuite/ver_test_pr16504_b.c [new file with mode: 0644]
gold/testsuite/ver_test_pr16504_b.script [new file with mode: 0644]

index bc17a9e..e38262e 100644 (file)
@@ -1,3 +1,22 @@
+2018-04-24  Cary Coutant  <ccoutant@gmail.com>
+
+       PR gold/16504
+       * dynobj.cc (Versions::symbol_section_contents): Don't set
+       VERSYM_HIDDEN flag for undefined symbols.
+       * symtab.cc (Symbol_table::add_from_object): Don't override default
+       version definition with a different default version.
+       * symtab.h (Symbol::from_dyn): New method.
+       * testsuite/plugin_test.c (struct sym_info): Add ver field.
+       (claim_file_hook): Pass symbol version to plugin API.
+       (parse_readelf_line): Parse symbol version.
+       * testsuite/Makefile.am (ver_test_pr16504): New test case.
+       * testsuite/Makefile.in: Regenerate.
+       * testsuite/ver_test_pr16504.sh: New test script.
+       * testsuite/ver_test_pr16504_a.c: New source file.
+       * testsuite/ver_test_pr16504_a.script: New version script.
+       * testsuite/ver_test_pr16504_b.c: New source file.
+       * testsuite/ver_test_pr16504_b.script: New version script.
+
 2018-04-19  Cary Coutant  <ccoutant@gmail.com>
 
        PR gold/23046
index d85adbc..7012802 100644 (file)
@@ -1776,7 +1776,10 @@ Versions::symbol_section_contents(const Symbol_table* symtab,
        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())
+      if ((*p)->version() != NULL
+         && (*p)->is_defined()
+         && !(*p)->is_default()
+         && !(*p)->from_dyn())
         version_index |= elfcpp::VERSYM_HIDDEN;
       elfcpp::Swap<16, big_endian>::writeval(pbuf + (*p)->dynsym_index() * 2,
                                              version_index);
index 34551ac..238834d 100644 (file)
@@ -989,7 +989,7 @@ Symbol_table::add_from_object(Object* object,
   // ins.first->second: the value (Symbol*).
   // ins.second: true if new entry was inserted, false if not.
 
-  Sized_symbol<size>* ret;
+  Sized_symbol<size>* ret = NULL;
   bool was_undefined_in_reg;
   bool was_common;
   if (!ins.second)
@@ -1049,17 +1049,42 @@ Symbol_table::add_from_object(Object* object,
          // it, then change it to NAME/VERSION.
          ret = this->get_sized_symbol<size>(insdefault.first->second);
 
-         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);
-          if (parameters->options().gc_sections())
-            this->gc_mark_dyn_syms(ret);
-         ins.first->second = ret;
+         // If the existing symbol already has a version,
+         // don't override it with the new symbol.
+         // This should only happen when the new symbol
+         // is from a shared library.
+         if (ret->version() != NULL)
+           {
+             if (!object->is_dynamic())
+               {
+                 gold_warning(_("%s: conflicting default version definition"
+                                " for %s@@%s"),
+                              object->name().c_str(), name, version);
+                 if (ret->source() == Symbol::FROM_OBJECT)
+                   gold_info(_("%s: %s: previous definition of %s@@%s here"),
+                             program_name,
+                             ret->object()->name().c_str(),
+                             name, ret->version());
+               }
+             ret = NULL;
+             is_default_version = false;
+           }
+         else
+           {
+             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);
+             if (parameters->options().gc_sections())
+               this->gc_mark_dyn_syms(ret);
+             ins.first->second = ret;
+           }
        }
-      else
+
+      if (ret == NULL)
        {
          was_undefined_in_reg = false;
          was_common = false;
index fdb7511..089e156 100644 (file)
@@ -344,6 +344,11 @@ class Symbol
   set_in_dyn()
   { this->in_dyn_ = true; }
 
+  // Return whether this symbol is defined in a dynamic object.
+  bool
+  from_dyn() const
+  { return this->source_ == FROM_OBJECT && this->object()->is_dynamic(); }
+
   // Return whether this symbol has been seen in a real ELF object.
   // (IN_REG will return TRUE if the symbol has been seen in either
   // a real ELF object or an object claimed by a plugin.)
index 7140df6..c926f8b 100644 (file)
@@ -2516,6 +2516,23 @@ plugin_pr22868_b_ir.o: plugin_pr22868_b.c
 plugin_pr22868_b.o: plugin_pr22868_b.c
        $(COMPILE) -c -fpic -o $@ $<
 
+check_SCRIPTS += ver_test_pr16504.sh
+check_DATA += ver_test_pr16504.stdout
+ver_test_pr16504.stdout: ver_test_pr16504.so
+       $(TEST_READELF) -W --dyn-syms $< >$@ 2>/dev/null
+ver_test_pr16504.so: ver_test_pr16504_a.so ver_test_pr16504_b.o.syms ver_test_pr16504_b.script gcctestdir/ld
+       gcctestdir/ld -shared -o $@ --plugin ./plugin_test.so --version-script $(srcdir)/ver_test_pr16504_b.script ver_test_pr16504_b.o.syms ver_test_pr16504_a.so
+ver_test_pr16504_a.so: ver_test_pr16504_a.o ver_test_pr16504_a.script gcctestdir/ld
+       gcctestdir/ld -shared -o $@ --version-script $(srcdir)/ver_test_pr16504_a.script ver_test_pr16504_a.o
+ver_test_pr16504_a.o: ver_test_pr16504_a.c
+       $(COMPILE) -c -fpic -o $@ $<
+# Filter out the UNDEFs from the symbols file to simulate GCC behavior,
+# which does not pass the UNDEF from a .symver directive.
+ver_test_pr16504_b.o.syms: ver_test_pr16504_b.o
+       $(TEST_READELF) -sW $< 2>/dev/null | grep -v "UND" >$@
+ver_test_pr16504_b.o: ver_test_pr16504_b.c
+       $(COMPILE) -c -fpic -o $@ $<
+
 endif PLUGINS
 
 check_PROGRAMS += exclude_libs_test
index 48e5b9e..1e60807 100644 (file)
@@ -612,7 +612,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_pr22868.stdout
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_53 = plugin_final_layout.sh \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_layout_with_alignment.sh \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_pr22868.sh
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_pr22868.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   ver_test_pr16504.sh
 
 # Uses the plugin_final_layout.sh script above to avoid duplication
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@am__append_54 = plugin_final_layout.stdout \
@@ -620,7 +621,8 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_layout_new_file.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_layout_new_file_readelf.stdout \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_layout_with_alignment.stdout \
-@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_pr22868.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   plugin_pr22868.stdout \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   ver_test_pr16504.stdout
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am__append_55 = exclude_libs_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ local_labels_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ discard_locals_test
@@ -5368,6 +5370,8 @@ plugin_layout_with_alignment.sh.log: plugin_layout_with_alignment.sh
        @p='plugin_layout_with_alignment.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 plugin_pr22868.sh.log: plugin_pr22868.sh
        @p='plugin_pr22868.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+ver_test_pr16504.sh.log: ver_test_pr16504.sh
+       @p='ver_test_pr16504.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 exclude_libs_test.sh.log: exclude_libs_test.sh
        @p='exclude_libs_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 discard_locals_test.sh.log: discard_locals_test.sh
@@ -7220,6 +7224,20 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -c -DIR -fpic -o $@ $<
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@plugin_pr22868_b.o: plugin_pr22868_b.c
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -c -fpic -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504.stdout: ver_test_pr16504.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(TEST_READELF) -W --dyn-syms $< >$@ 2>/dev/null
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504.so: ver_test_pr16504_a.so ver_test_pr16504_b.o.syms ver_test_pr16504_b.script gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   gcctestdir/ld -shared -o $@ --plugin ./plugin_test.so --version-script $(srcdir)/ver_test_pr16504_b.script ver_test_pr16504_b.o.syms ver_test_pr16504_a.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_a.so: ver_test_pr16504_a.o ver_test_pr16504_a.script gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   gcctestdir/ld -shared -o $@ --version-script $(srcdir)/ver_test_pr16504_a.script ver_test_pr16504_a.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_a.o: ver_test_pr16504_a.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -c -fpic -o $@ $<
+# Filter out the UNDEFs from the symbols file to simulate GCC behavior,
+# which does not pass the UNDEF from a .symver directive.
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_b.o.syms: ver_test_pr16504_b.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(TEST_READELF) -sW $< 2>/dev/null | grep -v "UND" >$@
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@ver_test_pr16504_b.o: ver_test_pr16504_b.c
+@GCC_TRUE@@NATIVE_LINKER_TRUE@@PLUGINS_TRUE@   $(COMPILE) -c -fpic -o $@ $<
 @GCC_TRUE@@NATIVE_LINKER_TRUE@exclude_libs_test.syms: exclude_libs_test
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_READELF) -sW $< >$@ 2>/dev/null
 @GCC_TRUE@@NATIVE_LINKER_TRUE@libexclude_libs_test_1.a: exclude_libs_test_1.o
index e80f926..a21abca 100644 (file)
@@ -46,6 +46,7 @@ struct sym_info
   char* vis;
   char* sect;
   char* name;
+  char* ver;
 };
 
 static struct claimed_file* first_claimed_file = NULL;
@@ -397,7 +398,14 @@ claim_file_hook (const struct ld_plugin_input_file* file, int* claimed)
           syms[nsyms].name = malloc(len + 1);
           strncpy(syms[nsyms].name, info.name, len + 1);
         }
-      syms[nsyms].version = NULL;
+      if (info.ver == NULL)
+        syms[nsyms].version = NULL;
+      else
+        {
+          len = strlen(info.ver);
+          syms[nsyms].version = malloc(len + 1);
+          strncpy(syms[nsyms].version, info.ver, len + 1);
+        }
       syms[nsyms].def = def;
       syms[nsyms].visibility = vis;
       syms[nsyms].size = info.size;
@@ -676,11 +684,26 @@ parse_readelf_line(char* p, struct sym_info* info)
   p += strspn(p, " ");
 
   /* Name field.  */
-  /* FIXME:  Look for version.  */
-  len = strlen(p);
-  if (len == 0)
-    p = NULL;
-  else if (p[len-1] == '\n')
-    p[--len] = '\0';
-  info->name = p;
+  len = strcspn(p, "@\n");
+  if (len > 0 && p[len] == '@')
+    {
+      /* Get the symbol version.  */
+      char* vp = p + len;
+      int vlen;
+
+      vp += strspn(vp, "@");
+      vlen = strcspn(vp, "\n");
+      vp[vlen] = '\0';
+      if (vlen > 0)
+       info->ver = vp;
+      else
+       info->ver = NULL;
+    }
+  else
+    info->ver = NULL;
+  p[len] = '\0';
+  if (len > 0)
+    info->name = p;
+  else
+    info->name = NULL;
 }
diff --git a/gold/testsuite/ver_test_pr16504.sh b/gold/testsuite/ver_test_pr16504.sh
new file mode 100755 (executable)
index 0000000..f4e3b8b
--- /dev/null
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+# ver_test_pr16504.sh -- test that undef gets correct version with LTO.
+
+# Copyright (C) 2018 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.
+
+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_pr16504.stdout " FUNC .* UND  *foo@VER2"
+check ver_test_pr16504.stdout " IFUNC .* foo@@VER1"
+
+exit 0
diff --git a/gold/testsuite/ver_test_pr16504_a.c b/gold/testsuite/ver_test_pr16504_a.c
new file mode 100644 (file)
index 0000000..7628022
--- /dev/null
@@ -0,0 +1,5 @@
+const char* foo(void);
+
+const char*
+foo(void)
+{ return "x"; }
diff --git a/gold/testsuite/ver_test_pr16504_a.script b/gold/testsuite/ver_test_pr16504_a.script
new file mode 100644 (file)
index 0000000..86bb0a0
--- /dev/null
@@ -0,0 +1,4 @@
+VER2 {
+global:
+  foo;
+};
diff --git a/gold/testsuite/ver_test_pr16504_b.c b/gold/testsuite/ver_test_pr16504_b.c
new file mode 100644 (file)
index 0000000..e6d82b6
--- /dev/null
@@ -0,0 +1,10 @@
+void
+new_foo(void);
+__asm__(".symver new_foo,foo@VER2");
+
+static void (*resolve_foo(void)) (void)
+{
+  return new_foo;
+}
+
+void foo(void) __attribute__((ifunc("resolve_foo")));
diff --git a/gold/testsuite/ver_test_pr16504_b.script b/gold/testsuite/ver_test_pr16504_b.script
new file mode 100644 (file)
index 0000000..94b373d
--- /dev/null
@@ -0,0 +1,4 @@
+VER1 {
+global:
+  foo;
+};