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]

Re: One and a half bugs plus some hacks


"Andreas Hartmetz" <ahartmetz@gmail.com> writes:

> 2008/5/7 Ian Lance Taylor <iant@google.com>:
>> "Andreas Hartmetz" <ahartmetz@gmail.com> writes:
>>
>>
>> > -gold aborts with an error message about "undefined symbol version
>>  > OPENSSL_0.9.8" (paraphrased) in one instance when linking against
>>  > openssl.
>>
>>  This one I don't understand.  Can you give me some more details about
>>  what is happening?

Andreas provided some additional details off-list--thanks!  I believe
the problem is that gold was not properly overriding the version when
it saw a normal definition of a symbol after seeing a definition of
the symbol in a shared library with a non-hidden version.

In the case at hand I suspect this is happening due to an over-zealous
a version script, which does "global: *;".  This is not a good idea--a
version script should control symbol visibility, which means that it
should explicitly list the global symbols and not just make all
symbols global.  However, that is a valid version script, and in fact
gold did not handle it correctly.  Most of this patch is actually
making that work.

Once that worked, I could recreate the problem using gold.  The few
lines in resolve.cc fix it.

Andreas, as far as I know, at this point all the problem with building
KDE are fixed.  Please let me know if there are problems.  And thanks
for reporting them.

Ian


	* symtab.c (Symbol::init_base_output_data): Add version
	parameter.  Change all callers.
	(Symbol::init_base_output_segment): Likewise.
	(Symbol::init_base_constant): Likewise.
	(Symbol::init_base_undefined): Likewise.
	(Sized_symbol::init_output_data): Likewise.
	(Sized_symbol::init_output_segment): Likewise.
	(Sized_symbol::init_constant): Likewise.
	(Sized_symbol::init_undefined): Likewise.
	(Symbol_table::do_define_in_output_data): If the new symbol has a
	version, mark it as the default.
	(Symbol_table::do_define_in_output_segment): Likewise.
	(Symbol_table::do_define_as_constant): Likewise.
	* symtab.h (class Symbol): Update declarations.
	(class Sized_symbol): Likewise.
	* resolve.cc (Symbol::override_version): New function.
	(Symbol::override_base: Call override_version.
	(Symbol::override_base_with_special): Likewise.
	* testsuite/ver_script_8.script: New file.
	* testsuite/Makefile.am (check_PROGRAMS): Add ver_test_8.
	(ver_test_8_SOURCES, ver_test_8_DEPENDENCIES): Define.
	(ver_test_8_LDFLAGS, ver_test_8_LDADD): Define.
	(ver_test_8_1.so, ver_test_8_2.so): New targets.


Index: resolve.cc
===================================================================
RCS file: /cvs/src/src/gold/resolve.cc,v
retrieving revision 1.35
diff -p -u -r1.35 resolve.cc
--- resolve.cc	7 May 2008 06:08:01 -0000	1.35
+++ resolve.cc	8 May 2008 18:38:18 -0000
@@ -32,6 +32,36 @@ namespace gold
 
 // Symbol methods used in this file.
 
+// This symbol is being overridden by another symbol whose version is
+// VERSION.  Update the VERSION_ field accordingly.
+
+inline void
+Symbol::override_version(const char* version)
+{
+  if (version == NULL)
+    {
+      // This is the case where this symbol is NAME/VERSION, and the
+      // version was not marked as hidden.  That makes it the default
+      // version, so we create NAME/NULL.  Later we see another symbol
+      // NAME/NULL, and that symbol is overriding this one.  In this
+      // case, since NAME/VERSION is the default, we make NAME/NULL
+      // override NAME/VERSION as well.  They are already the same
+      // Symbol structure.  Setting the VERSION_ field to NULL ensures
+      // that it will be output with the correct, empty, version.
+      this->version_ = version;
+    }
+  else
+    {
+      // This is the case where this symbol is NAME/VERSION_ONE, and
+      // now we see NAME/VERSION_TWO, and NAME/VERSION_TWO is
+      // overriding NAME.  If VERSION_ONE and VERSION_TWO are
+      // different, then this can only happen when VERSION_ONE is NULL
+      // and VERSION_TWO is not hidden.
+      gold_assert(this->version_ == version || this->version_ == NULL);
+      this->version_ = version;
+    }
+}
+
 // Override the fields in Symbol.
 
 template<int size, bool big_endian>
@@ -42,11 +72,7 @@ Symbol::override_base(const elfcpp::Sym<
 {
   gold_assert(this->source_ == FROM_OBJECT);
   this->u_.from_object.object = object;
-  if (version != NULL && this->version() != version)
-    {
-      gold_assert(this->version() == NULL);
-      this->version_ = version;
-    }
+  this->override_version(version);
   this->u_.from_object.shndx = st_shndx;
   this->is_ordinary_shndx_ = is_ordinary;
   this->type_ = sym.get_st_type();
@@ -673,12 +699,7 @@ Symbol::override_base_with_special(const
       break;
     }
 
-  if (from->version_ != NULL && this->version_ != from->version_)
-    {
-      gold_assert(this->version_ == NULL);
-      this->version_ = from->version_;
-    }
-
+  this->override_version(from->version_);
   this->type_ = from->type_;
   this->binding_ = from->binding_;
   this->visibility_ = from->visibility_;
Index: symtab.cc
===================================================================
RCS file: /cvs/src/src/gold/symtab.cc,v
retrieving revision 1.97
diff -p -u -r1.97 symtab.cc
--- symtab.cc	7 May 2008 06:08:01 -0000	1.97
+++ symtab.cc	8 May 2008 18:38:18 -0000
@@ -123,12 +123,12 @@ Symbol::init_base_object(const char* nam
 // in an Output_data.
 
 void
-Symbol::init_base_output_data(const char* name, Output_data* od,
-			      elfcpp::STT type, elfcpp::STB binding,
-			      elfcpp::STV visibility, unsigned char nonvis,
-			      bool offset_is_from_end)
+Symbol::init_base_output_data(const char* name, const char* version,
+			      Output_data* od, elfcpp::STT type,
+			      elfcpp::STB binding, elfcpp::STV visibility,
+			      unsigned char nonvis, bool offset_is_from_end)
 {
-  this->init_fields(name, NULL, type, binding, visibility, nonvis);
+  this->init_fields(name, version, type, binding, visibility, nonvis);
   this->u_.in_output_data.output_data = od;
   this->u_.in_output_data.offset_is_from_end = offset_is_from_end;
   this->source_ = IN_OUTPUT_DATA;
@@ -139,12 +139,13 @@ Symbol::init_base_output_data(const char
 // in an Output_segment.
 
 void
-Symbol::init_base_output_segment(const char* name, Output_segment* os,
-				 elfcpp::STT type, elfcpp::STB binding,
-				 elfcpp::STV visibility, unsigned char nonvis,
+Symbol::init_base_output_segment(const char* name, const char* version,
+				 Output_segment* os, elfcpp::STT type,
+				 elfcpp::STB binding, elfcpp::STV visibility,
+				 unsigned char nonvis,
 				 Segment_offset_base offset_base)
 {
-  this->init_fields(name, NULL, type, binding, visibility, nonvis);
+  this->init_fields(name, version, type, binding, visibility, nonvis);
   this->u_.in_output_segment.output_segment = os;
   this->u_.in_output_segment.offset_base = offset_base;
   this->source_ = IN_OUTPUT_SEGMENT;
@@ -155,11 +156,11 @@ Symbol::init_base_output_segment(const c
 // as a constant.
 
 void
-Symbol::init_base_constant(const char* name, elfcpp::STT type,
-			   elfcpp::STB binding, elfcpp::STV visibility,
-			   unsigned char nonvis)
+Symbol::init_base_constant(const char* name, const char* version,
+			   elfcpp::STT type, elfcpp::STB binding,
+			   elfcpp::STV visibility, unsigned char nonvis)
 {
-  this->init_fields(name, NULL, type, binding, visibility, nonvis);
+  this->init_fields(name, version, type, binding, visibility, nonvis);
   this->source_ = IS_CONSTANT;
   this->in_reg_ = true;
 }
@@ -168,11 +169,11 @@ Symbol::init_base_constant(const char* n
 // symbol.
 
 void
-Symbol::init_base_undefined(const char* name, elfcpp::STT type,
-			    elfcpp::STB binding, elfcpp::STV visibility,
-			    unsigned char nonvis)
+Symbol::init_base_undefined(const char* name, const char* version,
+			    elfcpp::STT type, elfcpp::STB binding,
+			    elfcpp::STV visibility, unsigned char nonvis)
 {
-  this->init_fields(name, NULL, type, binding, visibility, nonvis);
+  this->init_fields(name, version, type, binding, visibility, nonvis);
   this->source_ = IS_UNDEFINED;
   this->in_reg_ = true;
 }
@@ -208,15 +209,16 @@ Sized_symbol<size>::init_object(const ch
 
 template<int size>
 void
-Sized_symbol<size>::init_output_data(const char* name, Output_data* od,
-				     Value_type value, Size_type symsize,
-				     elfcpp::STT type, elfcpp::STB binding,
+Sized_symbol<size>::init_output_data(const char* name, const char* version,
+				     Output_data* od, Value_type value,
+				     Size_type symsize, elfcpp::STT type,
+				     elfcpp::STB binding,
 				     elfcpp::STV visibility,
 				     unsigned char nonvis,
 				     bool offset_is_from_end)
 {
-  this->init_base_output_data(name, od, type, binding, visibility, nonvis,
-			      offset_is_from_end);
+  this->init_base_output_data(name, version, od, type, binding, visibility,
+			      nonvis, offset_is_from_end);
   this->value_ = value;
   this->symsize_ = symsize;
 }
@@ -226,15 +228,16 @@ Sized_symbol<size>::init_output_data(con
 
 template<int size>
 void
-Sized_symbol<size>::init_output_segment(const char* name, Output_segment* os,
-					Value_type value, Size_type symsize,
-					elfcpp::STT type, elfcpp::STB binding,
+Sized_symbol<size>::init_output_segment(const char* name, const char* version,
+					Output_segment* os, Value_type value,
+					Size_type symsize, elfcpp::STT type,
+					elfcpp::STB binding,
 					elfcpp::STV visibility,
 					unsigned char nonvis,
 					Segment_offset_base offset_base)
 {
-  this->init_base_output_segment(name, os, type, binding, visibility, nonvis,
-				 offset_base);
+  this->init_base_output_segment(name, version, os, type, binding, visibility,
+				 nonvis, offset_base);
   this->value_ = value;
   this->symsize_ = symsize;
 }
@@ -244,12 +247,12 @@ Sized_symbol<size>::init_output_segment(
 
 template<int size>
 void
-Sized_symbol<size>::init_constant(const char* name, Value_type value,
-				  Size_type symsize, elfcpp::STT type,
-				  elfcpp::STB binding, elfcpp::STV visibility,
-				  unsigned char nonvis)
+Sized_symbol<size>::init_constant(const char* name, const char* version,
+				  Value_type value, Size_type symsize,
+				  elfcpp::STT type, elfcpp::STB binding,
+				  elfcpp::STV visibility, unsigned char nonvis)
 {
-  this->init_base_constant(name, type, binding, visibility, nonvis);
+  this->init_base_constant(name, version, type, binding, visibility, nonvis);
   this->value_ = value;
   this->symsize_ = symsize;
 }
@@ -258,11 +261,11 @@ Sized_symbol<size>::init_constant(const 
 
 template<int size>
 void
-Sized_symbol<size>::init_undefined(const char* name, elfcpp::STT type,
-				   elfcpp::STB binding, elfcpp::STV visibility,
-				   unsigned char nonvis)
+Sized_symbol<size>::init_undefined(const char* name, const char* version,
+				   elfcpp::STT type, elfcpp::STB binding,
+				   elfcpp::STV visibility, unsigned char nonvis)
 {
-  this->init_base_undefined(name, type, binding, visibility, nonvis);
+  this->init_base_undefined(name, version, type, binding, visibility, nonvis);
   this->value_ = 0;
   this->symsize_ = 0;
 }
@@ -1371,15 +1374,16 @@ Symbol_table::do_define_in_output_data(
   if (sym == NULL)
     return NULL;
 
-  gold_assert(version == NULL || oldsym != NULL);
-  sym->init_output_data(name, od, value, symsize, type, binding, visibility,
-			nonvis, offset_is_from_end);
+  sym->init_output_data(name, version, od, value, symsize, type, binding,
+			visibility, nonvis, offset_is_from_end);
 
   if (oldsym == NULL)
     {
       if (binding == elfcpp::STB_LOCAL
 	  || this->version_script_.symbol_is_local(name))
 	this->force_local(sym);
+      else if (version != NULL)
+	sym->set_is_default();
       return sym;
     }
 
@@ -1471,8 +1475,7 @@ Symbol_table::do_define_in_output_segmen
   if (sym == NULL)
     return NULL;
 
-  gold_assert(version == NULL || oldsym != NULL);
-  sym->init_output_segment(name, os, value, symsize, type, binding,
+  sym->init_output_segment(name, version, os, value, symsize, type, binding,
 			   visibility, nonvis, offset_base);
 
   if (oldsym == NULL)
@@ -1480,6 +1483,8 @@ Symbol_table::do_define_in_output_segmen
       if (binding == elfcpp::STB_LOCAL
 	  || this->version_script_.symbol_is_local(name))
 	this->force_local(sym);
+      else if (version != NULL)
+	sym->set_is_default();
       return sym;
     }
 
@@ -1571,8 +1576,8 @@ Symbol_table::do_define_as_constant(
   if (sym == NULL)
     return NULL;
 
-  gold_assert(version == NULL || version == name || oldsym != NULL);
-  sym->init_constant(name, value, symsize, type, binding, visibility, nonvis);
+  sym->init_constant(name, version, value, symsize, type, binding, visibility,
+		     nonvis);
 
   if (oldsym == NULL)
     {
@@ -1584,6 +1589,9 @@ Symbol_table::do_define_as_constant(
 	  && (binding == elfcpp::STB_LOCAL
 	      || this->version_script_.symbol_is_local(name)))
 	this->force_local(sym);
+      else if (version != NULL
+	       && (name != version || value != 0))
+	sym->set_is_default();
       return sym;
     }
 
@@ -1775,7 +1783,7 @@ Symbol_table::do_add_undefined_symbols_f
 
       gold_assert(oldsym == NULL);
 
-      sym->init_undefined(name, elfcpp::STT_NOTYPE, elfcpp::STB_GLOBAL,
+      sym->init_undefined(name, version, elfcpp::STT_NOTYPE, elfcpp::STB_GLOBAL,
 			  elfcpp::STV_DEFAULT, 0);
       ++this->saw_undefined_;
     }
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.77
diff -p -u -r1.77 symtab.h
--- symtab.h	7 May 2008 06:08:01 -0000	1.77
+++ symtab.h	8 May 2008 18:38:18 -0000
@@ -668,26 +668,29 @@ class Symbol
 
   // Initialize fields for an Output_data.
   void
-  init_base_output_data(const char* name, Output_data*, elfcpp::STT,
-			elfcpp::STB, elfcpp::STV, unsigned char nonvis,
-			bool offset_is_from_end);
+  init_base_output_data(const char* name, const char* version, Output_data*,
+			elfcpp::STT, elfcpp::STB, elfcpp::STV,
+			unsigned char nonvis, bool offset_is_from_end);
 
   // Initialize fields for an Output_segment.
   void
-  init_base_output_segment(const char* name, Output_segment* os,
-			   elfcpp::STT type, elfcpp::STB binding,
-			   elfcpp::STV visibility, unsigned char nonvis,
+  init_base_output_segment(const char* name, const char* version,
+			   Output_segment* os, elfcpp::STT type,
+			   elfcpp::STB binding, elfcpp::STV visibility,
+			   unsigned char nonvis,
 			   Segment_offset_base offset_base);
 
   // Initialize fields for a constant.
   void
-  init_base_constant(const char* name, elfcpp::STT type, elfcpp::STB binding,
-		     elfcpp::STV visibility, unsigned char nonvis);
+  init_base_constant(const char* name, const char* version, elfcpp::STT type,
+		     elfcpp::STB binding, elfcpp::STV visibility,
+		     unsigned char nonvis);
 
   // Initialize fields for an undefined symbol.
   void
-  init_base_undefined(const char* name, elfcpp::STT type, elfcpp::STB binding,
-		      elfcpp::STV visibility, unsigned char nonvis);
+  init_base_undefined(const char* name, const char* version, elfcpp::STT type,
+		      elfcpp::STB binding, elfcpp::STV visibility,
+		      unsigned char nonvis);
 
   // Override existing symbol.
   template<int size, bool big_endian>
@@ -699,6 +702,10 @@ class Symbol
   void
   override_base_with_special(const Symbol* from);
 
+  // Override symbol version.
+  void
+  override_version(const char* version);
+
   // Allocate a common symbol by giving it a location in the output
   // file.
   void
@@ -849,25 +856,28 @@ class Sized_symbol : public Symbol
 
   // Initialize fields for an Output_data.
   void
-  init_output_data(const char* name, Output_data*, Value_type value,
-		   Size_type symsize, elfcpp::STT, elfcpp::STB, elfcpp::STV,
-		   unsigned char nonvis, bool offset_is_from_end);
+  init_output_data(const char* name, const char* version, Output_data*,
+		   Value_type value, Size_type symsize, elfcpp::STT,
+		   elfcpp::STB, elfcpp::STV, unsigned char nonvis,
+		   bool offset_is_from_end);
 
   // Initialize fields for an Output_segment.
   void
-  init_output_segment(const char* name, Output_segment*, Value_type value,
-		      Size_type symsize, elfcpp::STT, elfcpp::STB, elfcpp::STV,
-		      unsigned char nonvis, Segment_offset_base offset_base);
+  init_output_segment(const char* name, const char* version, Output_segment*,
+		      Value_type value, Size_type symsize, elfcpp::STT,
+		      elfcpp::STB, elfcpp::STV, unsigned char nonvis,
+		      Segment_offset_base offset_base);
 
   // Initialize fields for a constant.
   void
-  init_constant(const char* name, Value_type value, Size_type symsize,
-		elfcpp::STT, elfcpp::STB, elfcpp::STV, unsigned char nonvis);
+  init_constant(const char* name, const char* version, Value_type value,
+		Size_type symsize, elfcpp::STT, elfcpp::STB, elfcpp::STV,
+		unsigned char nonvis);
 
   // Initialize fields for an undefined symbol.
   void
-  init_undefined(const char* name, elfcpp::STT, elfcpp::STB, elfcpp::STV,
-		 unsigned char nonvis);
+  init_undefined(const char* name, const char* version, elfcpp::STT,
+		 elfcpp::STB, elfcpp::STV, unsigned char nonvis);
 
   // Override existing symbol.
   template<bool big_endian>
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.68
diff -p -u -r1.68 Makefile.am
--- testsuite/Makefile.am	6 May 2008 22:24:26 -0000	1.68
+++ testsuite/Makefile.am	8 May 2008 18:38:19 -0000
@@ -750,6 +750,16 @@ check_DATA += ver_test_7.syms
 ver_test_7.syms: ver_test_7.so
 	$(TEST_READELF) -s $< >$@ 2>/dev/null
 
+check_PROGRAMS += ver_test_8
+ver_test_8_SOURCES = two_file_test_main.cc
+ver_test_8_DEPENDENCIES = gcctestdir/ld ver_test_8_1.so ver_test_8_2.so
+ver_test_8_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
+ver_test_8_LDADD = ver_test_8_1.so ver_test_8_2.so
+ver_test_8_1.so: two_file_test_1_pic.o two_file_test_1b_pic.o ver_test_8_2.so gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_1b_pic.o ver_test_8_2.so
+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_PROGRAMS += protected_1
 protected_1_SOURCES = \
 	protected_main_1.cc protected_main_2.cc protected_main_3.cc
Index: testsuite/ver_test_8.script
===================================================================
RCS file: testsuite/ver_test_8.script
diff -N testsuite/ver_test_8.script
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/ver_test_8.script	8 May 2008 18:38:19 -0000
@@ -0,0 +1,26 @@
+## ver_test_8.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.
+
+VER_TEST_8 {
+  global:
+    *;
+};

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