* symtab.cc (Symbol_table::add_from_object): Rewrite the case

where we see NAME/NULL and NAME/VERSION  as separate symbols.
	* testsuite/ver_test_main.cc (main): Call t4.
	(t4, t4_2a): Define.
	* testsuite/ver_test_2.cc (t4_2): Define.
	* testsuite/ver_test_2.script: Put t4_2a in VER2.
	* testsuite/ver_test_4.cc (t4_2a): Define.
	* testsuite/ver_test_4.script: Put t4_2a in VER2.
	* testsuite/ver_test.h (t4, t4_2, t4_2a): Declare.

gold/ChangeLog

@@ -1,3 +1,15 @@
+2008-07-18  Ian Lance Taylor  <iant@google.com>
+
+	* symtab.cc (Symbol_table::add_from_object): Rewrite the case
+	where we see NAME/NULL and NAME/VERSION  as separate symbols.
+	* testsuite/ver_test_main.cc (main): Call t4.
+	(t4, t4_2a): Define.
+	* testsuite/ver_test_2.cc (t4_2): Define.
+	* testsuite/ver_test_2.script: Put t4_2a in VER2.
+	* testsuite/ver_test_4.cc (t4_2a): Define.
+	* testsuite/ver_test_4.script: Put t4_2a in VER2.
+	* testsuite/ver_test.h (t4, t4_2, t4_2a): Declare.
+
 2008-07-17  Ian Lance Taylor  <iant@google.com>
 
 	* dynobj.cc (Versions::add_def): If we give an error about a

gold/symtab.cc

@@ -687,23 +687,58 @@ Symbol_table::add_from_object(Object* object,
 	      // NAME/NULL point to NAME/VERSION.
 	      insdef.first->second = ret;
 	    }
-	  else if (insdef.first->second != ret
-	           && insdef.first->second->is_undefined())
+	  else if (insdef.first->second != ret)
 	    {
 	      // This is the unfortunate case where we already have
-	      // entries for both NAME/VERSION and NAME/NULL.  Note
-	      // that we don't want to combine them if the existing
-	      // symbol is going to override the new one.  FIXME: We
-	      // currently just test is_undefined, but this may not do
-	      // the right thing if the existing symbol is from a
-	      // shared library and the new one is from a regular
-	      // object.
+	      // entries for both NAME/VERSION and NAME/NULL.  We now
+	      // see a symbol NAME/VERSION where VERSION is the
+	      // default version.  We have already resolved this new
+	      // symbol with the existing NAME/VERSION symbol.
 
-	      const Sized_symbol<size>* sym2;
-	      sym2 = this->get_sized_symbol<size>(insdef.first->second);
-	      Symbol_table::resolve<size, big_endian>(ret, sym2, version);
-	      this->make_forwarder(insdef.first->second, ret);
-	      insdef.first->second = ret;
+	      // It's possible that NAME/NULL and NAME/VERSION are
+	      // both defined in regular objects.  This can only
+	      // happen if one object file defines foo and another
+	      // defines foo@@ver.  This is somewhat obscure, but we
+	      // call it a multiple definition error.
+
+	      // It's possible that NAME/NULL actually has a version,
+	      // in which case it won't be the same as VERSION.  This
+	      // happens with ver_test_7.so in the testsuite for the
+	      // symbol t2_2.  We see t2_2@@VER2, so we define both
+	      // t2_2/VER2 and t2_2/NULL.  We then see an unadorned
+	      // t2_2 in an object file and give it version VER1 from
+	      // the version script.  This looks like a default
+	      // definition for VER1, so it looks like we should merge
+	      // t2_2/NULL with t2_2/VER1.  That doesn't make sense,
+	      // but it's not obvious that this is an error, either.
+	      // So we just punt.
+
+	      // If one of the symbols has non-default visibility, and
+	      // the other is defined in a shared object, then they
+	      // are different symbols.
+
+	      // Otherwise, we just resolve the symbols as though they
+	      // were the same.
+
+	      if (insdef.first->second->version() != NULL)
+		{
+		  gold_assert(insdef.first->second->version() != version);
+		  def = false;
+		}
+	      else if (ret->visibility() != elfcpp::STV_DEFAULT
+		  && insdef.first->second->is_from_dynobj())
+		def = false;
+	      else if (insdef.first->second->visibility() != elfcpp::STV_DEFAULT
+		       && ret->is_from_dynobj())
+		def = false;
+	      else
+		{
+		  const Sized_symbol<size>* sym2;
+		  sym2 = this->get_sized_symbol<size>(insdef.first->second);
+		  Symbol_table::resolve<size, big_endian>(ret, sym2, version);
+		  this->make_forwarder(insdef.first->second, ret);
+		  insdef.first->second = ret;
+		}
 	    }
 	  else
 	    def = false;

gold/testsuite/ver_test.h

@@ -30,11 +30,14 @@
 extern bool t1();
 extern bool t2();
 extern bool t3();
+extern bool t4();
 
 extern "C" {
 
 extern int t1_2();
 extern int t2_2();
 extern int t3_2();
+extern int t4_2();
+extern int t4_2a();
 
 }

gold/testsuite/ver_test_2.cc

@@ -28,3 +28,13 @@ t3_2()
   TRACE
   return t1_2();
 }
+
+// Calls a versioned function in ver_test_4.cc which should be
+// overridden by an unversioned function in the main program.
+
+int
+t4_2()
+{
+  TRACE
+  return t4_2a();
+}

gold/testsuite/ver_test_2.script

@@ -27,5 +27,5 @@ VER2 {
   global:
     t1_2;
     t3_2;
+    t4_2a;
 } VER1;
-

gold/testsuite/ver_test_4.cc

@@ -51,3 +51,14 @@ t2_2_b()
   TRACE
   return 22;
 }
+
+
+// This function is given a version by the version script, and should
+// be overridden by the main program.
+
+int
+t4_2a()
+{
+  TRACE
+  return -42;
+}

gold/testsuite/ver_test_4.script

@@ -31,5 +31,6 @@ VER2 {
   global:
     t1_2;
     t2_2;
+    t4_2a;
 } VER1;
 

gold/testsuite/ver_test_main.cc

@@ -30,6 +30,7 @@ main()
   assert(t1());
   assert(t2());
   assert(t3());
+  assert(t4());
   return 0;
 }
 
@@ -52,3 +53,22 @@ t3()
   TRACE
   return t3_2() == 12;
 }
+
+// Call a function in a shared library that calls a function which is
+// defined in the main program and defined with a default version in
+// the shared library.  The symbol in the main program should override
+// even though it doesn't have a version.
+
+bool
+t4()
+{
+  TRACE
+  return t4_2() == 42;
+}
+
+int
+t4_2a()
+{
+  TRACE
+  return 42;
+}