This is the mail archive of the binutils-cvs@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]

[binutils-gdb] Fix problem where version script causes predefined hidden symbol to be defined twice.


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=40d7d93ff412f4c34cde3daa04890d5cd2e0d9c9

commit 40d7d93ff412f4c34cde3daa04890d5cd2e0d9c9
Author: Cary Coutant <ccoutant@gmail.com>
Date:   Wed Dec 21 17:32:39 2016 -0800

    Fix problem where version script causes predefined hidden symbol to be defined twice.
    
    When creating a predefined hidden symbol like _GLOBAL_OFFSET_TABLE_, gold
    was incorrectly letting a version script add a version to the symbol,
    resulting in two copies of the symbol, both STB_LOCAL, but one of which
    was grouped in the globals part of the symbol table.
    
    gold/
    	* symtab.cc (Symbol_table::define_special_symbol): Add is_forced_local
    	parameter; if set, do not check version script.
    	(Symbol_table::do_define_in_output_data): Pass is_forced_local for
    	STB_LOCAL predefined symbols.
    	(Symbol_table::do_define_in_output_segment): Likewise.
    	(Symbol_table::do_define_in_output_segment): Likewise.
    	(Symbol_table::do_define_as_constant): Likewise.
    	* symtab.h (Symbol_table::define_special_symbol): Add is_forced_local
    	parameter. Adjust all callers.
    	* testsuite/Makefile.am (ver_test_8.sh): New test case.
    	* testsuite/Makefile.in: Regenerate.
    	* ver_test_8.sh: New test script.

Diff:
---
 gold/ChangeLog               | 15 ++++++++++++++
 gold/symtab.cc               | 49 ++++++++++++++++++++++++--------------------
 gold/symtab.h                |  2 +-
 gold/testsuite/Makefile.am   |  5 +++++
 gold/testsuite/Makefile.in   | 13 +++++++++---
 gold/testsuite/ver_test_8.sh | 32 +++++++++++++++++++++++++++++
 6 files changed, 90 insertions(+), 26 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index ba74545..9ea609e 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,5 +1,20 @@
 2016-12-21  Cary Coutant  <ccoutant@gmail.com>
 
+	* symtab.cc (Symbol_table::define_special_symbol): Add is_forced_local
+	parameter; if set, do not check version script.
+	(Symbol_table::do_define_in_output_data): Pass is_forced_local for
+	STB_LOCAL predefined symbols.
+	(Symbol_table::do_define_in_output_segment): Likewise.
+	(Symbol_table::do_define_in_output_segment): Likewise.
+	(Symbol_table::do_define_as_constant): Likewise.
+	* symtab.h (Symbol_table::define_special_symbol): Add is_forced_local
+	parameter. Adjust all callers.
+	* testsuite/Makefile.am (ver_test_8.sh): New test case.
+	* testsuite/Makefile.in: Regenerate.
+	* ver_test_8.sh: New test script.
+
+2016-12-21  Cary Coutant  <ccoutant@gmail.com>
+
 	* output.cc (Output_segment::first_section): Return NULL if there are
 	no sections in the segment.
 	* output.h (Output_segment::first_section_load_address): Assert that
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 35989f0..d97fbdd 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1759,7 +1759,7 @@ Sized_symbol<size>*
 Symbol_table::define_special_symbol(const char** pname, const char** pversion,
 				    bool only_if_ref,
                                     Sized_symbol<size>** poldsym,
-				    bool* resolve_oldsym)
+				    bool* resolve_oldsym, bool is_forced_local)
 {
   *resolve_oldsym = false;
   *poldsym = NULL;
@@ -1768,7 +1768,7 @@ Symbol_table::define_special_symbol(const char** pname, const char** pversion,
   // the version script.
   std::string v;
   bool is_default_version = false;
-  if (*pversion == NULL)
+  if (!is_forced_local && *pversion == NULL)
     {
       bool is_global;
       if (this->version_script_.get_symbol_version(*pname, &v, &is_global))
@@ -1966,13 +1966,15 @@ Symbol_table::do_define_in_output_data(
   Sized_symbol<size>* sym;
   Sized_symbol<size>* oldsym;
   bool resolve_oldsym;
+  const bool is_forced_local = binding == elfcpp::STB_LOCAL;
 
   if (parameters->target().is_big_endian())
     {
 #if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
       sym = this->define_special_symbol<size, true>(&name, &version,
 						    only_if_ref, &oldsym,
-						    &resolve_oldsym);
+						    &resolve_oldsym,
+						    is_forced_local);
 #else
       gold_unreachable();
 #endif
@@ -1982,7 +1984,8 @@ Symbol_table::do_define_in_output_data(
 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
       sym = this->define_special_symbol<size, false>(&name, &version,
 						     only_if_ref, &oldsym,
-						     &resolve_oldsym);
+						     &resolve_oldsym,
+						     is_forced_local);
 #else
       gold_unreachable();
 #endif
@@ -1997,8 +2000,7 @@ Symbol_table::do_define_in_output_data(
 
   if (oldsym == NULL)
     {
-      if (binding == elfcpp::STB_LOCAL
-	  || this->version_script_.symbol_is_local(name))
+      if (is_forced_local || this->version_script_.symbol_is_local(name))
 	this->force_local(sym);
       else if (version != NULL)
 	sym->set_is_default();
@@ -2013,8 +2015,7 @@ Symbol_table::do_define_in_output_data(
   else
     {
       if (defined == PREDEFINED
-	  && (binding == elfcpp::STB_LOCAL
-	      || this->version_script_.symbol_is_local(name)))
+	  && (is_forced_local || this->version_script_.symbol_is_local(name)))
 	this->force_local(oldsym);
       delete sym;
       return oldsym;
@@ -2084,13 +2085,15 @@ Symbol_table::do_define_in_output_segment(
   Sized_symbol<size>* sym;
   Sized_symbol<size>* oldsym;
   bool resolve_oldsym;
+  const bool is_forced_local = binding == elfcpp::STB_LOCAL;
 
   if (parameters->target().is_big_endian())
     {
 #if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
       sym = this->define_special_symbol<size, true>(&name, &version,
 						    only_if_ref, &oldsym,
-						    &resolve_oldsym);
+						    &resolve_oldsym,
+						    is_forced_local);
 #else
       gold_unreachable();
 #endif
@@ -2100,7 +2103,8 @@ Symbol_table::do_define_in_output_segment(
 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
       sym = this->define_special_symbol<size, false>(&name, &version,
 						     only_if_ref, &oldsym,
-						     &resolve_oldsym);
+						     &resolve_oldsym,
+						     is_forced_local);
 #else
       gold_unreachable();
 #endif
@@ -2115,8 +2119,7 @@ Symbol_table::do_define_in_output_segment(
 
   if (oldsym == NULL)
     {
-      if (binding == elfcpp::STB_LOCAL
-	  || this->version_script_.symbol_is_local(name))
+      if (is_forced_local || this->version_script_.symbol_is_local(name))
 	this->force_local(sym);
       else if (version != NULL)
 	sym->set_is_default();
@@ -2130,8 +2133,7 @@ Symbol_table::do_define_in_output_segment(
     return sym;
   else
     {
-      if (binding == elfcpp::STB_LOCAL
-	  || this->version_script_.symbol_is_local(name))
+      if (is_forced_local || this->version_script_.symbol_is_local(name))
 	this->force_local(oldsym);
       delete sym;
       return oldsym;
@@ -2200,13 +2202,15 @@ Symbol_table::do_define_as_constant(
   Sized_symbol<size>* sym;
   Sized_symbol<size>* oldsym;
   bool resolve_oldsym;
+  const bool is_forced_local = binding == elfcpp::STB_LOCAL;
 
   if (parameters->target().is_big_endian())
     {
 #if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
       sym = this->define_special_symbol<size, true>(&name, &version,
 						    only_if_ref, &oldsym,
-						    &resolve_oldsym);
+						    &resolve_oldsym,
+						    is_forced_local);
 #else
       gold_unreachable();
 #endif
@@ -2216,7 +2220,8 @@ Symbol_table::do_define_as_constant(
 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
       sym = this->define_special_symbol<size, false>(&name, &version,
 						     only_if_ref, &oldsym,
-						     &resolve_oldsym);
+						     &resolve_oldsym,
+						     is_forced_local);
 #else
       gold_unreachable();
 #endif
@@ -2235,8 +2240,7 @@ Symbol_table::do_define_as_constant(
       if ((version == NULL
 	   || name != version
 	   || value != 0)
-	  && (binding == elfcpp::STB_LOCAL
-	      || this->version_script_.symbol_is_local(name)))
+	  && (is_forced_local || this->version_script_.symbol_is_local(name)))
 	this->force_local(sym);
       else if (version != NULL
 	       && (name != version || value != 0))
@@ -2252,8 +2256,7 @@ Symbol_table::do_define_as_constant(
     return sym;
   else
     {
-      if (binding == elfcpp::STB_LOCAL
-	  || this->version_script_.symbol_is_local(name))
+      if (is_forced_local || this->version_script_.symbol_is_local(name))
 	this->force_local(oldsym);
       delete sym;
       return oldsym;
@@ -2444,7 +2447,8 @@ Symbol_table::add_undefined_symbol_from_command_line(const char* name)
 #if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
       sym = this->define_special_symbol<size, true>(&name, &version,
 						    false, &oldsym,
-						    &resolve_oldsym);
+						    &resolve_oldsym,
+						    false);
 #else
       gold_unreachable();
 #endif
@@ -2454,7 +2458,8 @@ Symbol_table::add_undefined_symbol_from_command_line(const char* name)
 #if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
       sym = this->define_special_symbol<size, false>(&name, &version,
 						     false, &oldsym,
-						     &resolve_oldsym);
+						     &resolve_oldsym,
+						     false);
 #else
       gold_unreachable();
 #endif
diff --git a/gold/symtab.h b/gold/symtab.h
index b26b4e0..d0be59f 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1790,7 +1790,7 @@ class Symbol_table
   Sized_symbol<size>*
   define_special_symbol(const char** pname, const char** pversion,
 			bool only_if_ref, Sized_symbol<size>** poldsym,
-			bool* resolve_oldsym);
+			bool* resolve_oldsym, bool is_forced_local);
 
   // Define a symbol in an Output_data, sized version.
   template<int size>
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 7d91575..fbdc147 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1739,6 +1739,11 @@ ver_test_8_1.so: two_file_test_1_pic.o two_file_test_1b_pic.o ver_test_8_2.so gc
 ver_test_8_2.so: two_file_test_2_pic.o $(srcdir)/ver_test_8.script gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -shared -Wl,--version-script,$(srcdir)/ver_test_8.script two_file_test_2_pic.o
 
+check_SCRIPTS += ver_test_8.sh
+check_DATA += ver_test_8_2.so.syms
+ver_test_8_2.so.syms: ver_test_8_2.so
+	$(TEST_READELF) -s $< >$@ 2>/dev/null
+
 check_PROGRAMS += ver_test_9
 ver_test_9_SOURCES = ver_test_main.cc
 ver_test_9_DEPENDENCIES = gcctestdir/ld ver_test_9.so
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index a385ad4..d1ce1fb 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -409,8 +409,9 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	undef_symbol.sh pr18689.sh \
 @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@	ver_test_13.sh relro_test.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_7.sh ver_test_8.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_10.sh ver_test_13.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	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 \
@@ -461,7 +462,9 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	flagstest_o_ttext_2 \
 @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@	ver_test_7.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_8_2.so.syms \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	ver_test_10.syms \
 @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 \
@@ -5118,6 +5121,8 @@ ver_test_5.sh.log: ver_test_5.sh
 	@p='ver_test_5.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
 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_8.sh.log: ver_test_8.sh
+	@p='ver_test_8.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
@@ -6578,6 +6583,8 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_1b_pic.o ver_test_8_2.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_8_2.so: two_file_test_2_pic.o $(srcdir)/ver_test_8.script gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared -Wl,--version-script,$(srcdir)/ver_test_8.script two_file_test_2_pic.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_8_2.so.syms: ver_test_8_2.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(TEST_READELF) -s $< >$@ 2>/dev/null
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_9.so: ver_test_9.o ver_test_4.so ver_test_5.so gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared -Wl,-R,. ver_test_9.o ver_test_5.so ver_test_4.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ver_test_9.o: ver_test_9.cc
diff --git a/gold/testsuite/ver_test_8.sh b/gold/testsuite/ver_test_8.sh
new file mode 100755
index 0000000..4f46e3c
--- /dev/null
+++ b/gold/testsuite/ver_test_8.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+# ver_test_8.sh -- check that __GLOBAL_OFFSET_TABLE__ is defined only once.
+
+# Copyright (C) 2016 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.
+
+count=`grep -c '_GLOBAL_OFFSET_TABLE_' ver_test_8_2.so.syms`
+
+if test "$count" -ne 1; then
+  echo "Found $count copies of '_GLOBAL_OFFSET_TABLE_' (should be only 1)"
+  exit 1
+fi
+
+exit 0


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