From Cary Coutant: Fix mixing PIC and non-PIC relocs in the same

shared library.

8 files changed, 105 insertions, 32 deletions

diff --git a/gold/i386.cc b/gold/i386.cc
index 6c35940..fe8341d 100644
--- a/gold/i386.cc
+++ b/gold/i386.cc
@@ -164,8 +164,7 @@ class Target_i386 : public Sized_target<32, false>
     // Return whether the static relocation needs to be applied.
     inline bool
     should_apply_static_reloc(const Sized_symbol<32>* gsym,
-                              bool is_absolute_ref,
-                              bool is_function_call,
+                              int ref_flags,
                               bool is_32bit);
 
     // Do a relocation.  Return false if the caller should not issue
@@ -1094,7 +1093,7 @@ Target_i386::Scan::global(const General_options& options,
               gsym->set_needs_dynsym_value();
           }
         // Make a dynamic relocation if necessary.
-        if (gsym->needs_dynamic_reloc(true, false))
+        if (gsym->needs_dynamic_reloc(Symbol::ABSOLUTE_REF))
           {
             if (target->may_need_copy_reloc(gsym))
               {
@@ -1137,8 +1136,10 @@ Target_i386::Scan::global(const General_options& options,
               target->make_plt_entry(symtab, layout, gsym);
           }
         // Make a dynamic relocation if necessary.
-        bool is_function_call = (gsym->type() == elfcpp::STT_FUNC);
-        if (gsym->needs_dynamic_reloc(false, is_function_call))
+        int flags = Symbol::NON_PIC_REF;
+        if (gsym->type() == elfcpp::STT_FUNC)
+          flags |= Symbol::FUNCTION_CALL;
+        if (gsym->needs_dynamic_reloc(flags))
           {
             if (target->may_need_copy_reloc(gsym))
               {
@@ -1443,8 +1444,7 @@ Target_i386::do_finalize_sections(Layout* layout)
 
 inline bool
 Target_i386::Relocate::should_apply_static_reloc(const Sized_symbol<32>* gsym,
-                                                 bool is_absolute_ref,
-                                                 bool is_function_call,
+                                                 int ref_flags,
                                                  bool is_32bit)
 {
   // For local symbols, we will have created a non-RELATIVE dynamic
@@ -1453,12 +1453,18 @@ Target_i386::Relocate::should_apply_static_reloc(const Sized_symbol<32>* gsym,
   // (c) the relocation is not 32 bits wide.
   if (gsym == NULL)
     return !(parameters->output_is_position_independent()
-             && is_absolute_ref
+             && (ref_flags & Symbol::ABSOLUTE_REF)
              && !is_32bit);
 
-  // For global symbols, we use the same helper routines used in the scan pass.
-  return !(gsym->needs_dynamic_reloc(is_absolute_ref, is_function_call)
-           && !gsym->can_use_relative_reloc(is_function_call));
+  // For global symbols, we use the same helper routines used in the
+  // scan pass.  If we did not create a dynamic relocation, or if we
+  // created a RELATIVE dynamic relocation, we should apply the static
+  // relocation.
+  bool has_dyn = gsym->needs_dynamic_reloc(ref_flags);
+  bool is_rel = (ref_flags & Symbol::ABSOLUTE_REF)
+                && gsym->can_use_relative_reloc(ref_flags
+                                                & Symbol::FUNCTION_CALL);
+  return !has_dyn || is_rel;
 }
 
 // Perform a relocation.
@@ -1491,11 +1497,15 @@ Target_i386::Relocate::relocate(const Relocate_info<32, false>* relinfo,
 
   // Pick the value to use for symbols defined in shared objects.
   Symbol_value<32> symval;
+  bool is_nonpic = (r_type == elfcpp::R_386_PC8
+                    || r_type == elfcpp::R_386_PC16
+                    || r_type == elfcpp::R_386_PC32);
   if (gsym != NULL
       && (gsym->is_from_dynobj()
           || (parameters->output_is_shared()
               && gsym->is_preemptible()))
-      && gsym->has_plt_offset())
+      && gsym->has_plt_offset()
+      && (!is_nonpic || !parameters->output_is_shared()))
     {
       symval.set_output_value(target->plt_section()->address()
 			      + gsym->plt_offset());
@@ -1539,43 +1549,46 @@ Target_i386::Relocate::relocate(const Relocate_info<32, false>* relinfo,
       break;
 
     case elfcpp::R_386_32:
-      if (should_apply_static_reloc(gsym, true, false, true))
+      if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, true))
         Relocate_functions<32, false>::rel32(view, object, psymval);
       break;
 
     case elfcpp::R_386_PC32:
       {
-        bool is_function_call = (gsym != NULL
-                                 && gsym->type() == elfcpp::STT_FUNC);
-        if (should_apply_static_reloc(gsym, false, is_function_call, true))
+        int ref_flags = Symbol::NON_PIC_REF;
+        if (gsym != NULL && gsym->type() == elfcpp::STT_FUNC)
+          ref_flags |= Symbol::FUNCTION_CALL;
+        if (should_apply_static_reloc(gsym, ref_flags, true))
           Relocate_functions<32, false>::pcrel32(view, object, psymval, address);
       }
       break;
 
     case elfcpp::R_386_16:
-      if (should_apply_static_reloc(gsym, true, false, false))
+      if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, false))
         Relocate_functions<32, false>::rel16(view, object, psymval);
       break;
 
     case elfcpp::R_386_PC16:
       {
-        bool is_function_call = (gsym != NULL
-                                 && gsym->type() == elfcpp::STT_FUNC);
-        if (should_apply_static_reloc(gsym, false, is_function_call, false))
+        int ref_flags = Symbol::NON_PIC_REF;
+        if (gsym != NULL && gsym->type() == elfcpp::STT_FUNC)
+          ref_flags |= Symbol::FUNCTION_CALL;
+        if (should_apply_static_reloc(gsym, ref_flags, false))
           Relocate_functions<32, false>::pcrel32(view, object, psymval, address);
       }
       break;
 
     case elfcpp::R_386_8:
-      if (should_apply_static_reloc(gsym, true, false, false))
+      if (should_apply_static_reloc(gsym, Symbol::ABSOLUTE_REF, false))
         Relocate_functions<32, false>::rel8(view, object, psymval);
       break;
 
     case elfcpp::R_386_PC8:
       {
-        bool is_function_call = (gsym != NULL
-                                 && gsym->type() == elfcpp::STT_FUNC);
-        if (should_apply_static_reloc(gsym, false, is_function_call, false))
+        int ref_flags = Symbol::NON_PIC_REF;
+        if (gsym != NULL && gsym->type() == elfcpp::STT_FUNC)
+          ref_flags |= Symbol::FUNCTION_CALL;
+        if (should_apply_static_reloc(gsym, ref_flags, false))
           Relocate_functions<32, false>::pcrel32(view, object, psymval, address);
       }
       break;
diff --git a/gold/symtab.h b/gold/symtab.h
index 43a228d..d3e076b 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -480,21 +480,45 @@ class Symbol
             && (this->is_from_dynobj() || this->is_preemptible()));
   }
 
+  // When determining whether a reference to a symbol needs a dynamic
+  // relocation, we need to know several things about the reference.
+  // These flags may be or'ed together.
+  enum Reference_flags
+  {
+    // Reference to the symbol's absolute address.
+    ABSOLUTE_REF = 1,
+    // A non-PIC reference.
+    NON_PIC_REF = 2,
+    // A function call.
+    FUNCTION_CALL = 4
+  };
+
   // Given a direct absolute or pc-relative static relocation against
   // the global symbol, this function returns whether a dynamic relocation
   // is needed.
 
   bool
-  needs_dynamic_reloc(bool is_absolute_ref, bool is_function_call) const
+  needs_dynamic_reloc(int flags) const
   {
     // An absolute reference within a position-independent output file
-    // will need a dynamic relocaion.
-    if (is_absolute_ref && parameters->output_is_position_independent())
+    // will need a dynamic relocation.
+    if ((flags & ABSOLUTE_REF)
+        && parameters->output_is_position_independent())
       return true;
 
     // A function call that can branch to a local PLT entry does not need
     // a dynamic relocation.
-    if (is_function_call && this->has_plt_offset())
+    if ((flags & FUNCTION_CALL) && this->has_plt_offset())
+      return false;
+
+    // A non-pic pc-relative function call in a shared library whose target
+    // is defined in the same load module does not need a dynamic relocation.
+    // Even if the target is preemptible, we will bind directly, since we
+    // cannot use a PLT entry in this case.
+    if ((flags & FUNCTION_CALL)
+        && (flags & NON_PIC_REF)
+        && this->is_defined()
+        && parameters->output_is_shared())
       return false;
 
     // A reference to any PLT entry in a non-position-independent executable
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index 36c3c00..78f5472 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -140,6 +140,7 @@ check_PROGRAMS += two_file_shared_2_test
 check_PROGRAMS += two_file_shared_1_pic_2_test
 check_PROGRAMS += two_file_shared_2_pic_1_test
 check_PROGRAMS += two_file_same_shared_test
+check_PROGRAMS += two_file_mixed_shared_test
 check_PROGRAMS += two_file_separate_shared_12_test
 check_PROGRAMS += two_file_separate_shared_21_test
 two_file_test_1_pic.o: two_file_test_1.cc
@@ -152,6 +153,8 @@ two_file_shared_2.so: two_file_test_2_pic.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_2_pic.o
 two_file_shared.so: two_file_test_1_pic.o two_file_test_2_pic.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_2_pic.o
+two_file_shared_mixed.so: two_file_test_1_pic.o two_file_test_2.o gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_2.o
 
 two_file_shared_1_test_SOURCES = two_file_test_2.cc two_file_test_main.cc
 two_file_shared_1_test_DEPENDENCIES = gcctestdir/ld two_file_shared_1.so
@@ -180,6 +183,11 @@ two_file_same_shared_test_DEPENDENCIES = gcctestdir/ld two_file_shared.so
 two_file_same_shared_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
 two_file_same_shared_test_LDADD = two_file_shared.so
 
+two_file_mixed_shared_test_SOURCES = two_file_test_main.cc
+two_file_mixed_shared_test_DEPENDENCIES = gcctestdir/ld two_file_shared_mixed.so
+two_file_mixed_shared_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
+two_file_mixed_shared_test_LDADD = two_file_shared_mixed.so
+
 two_file_separate_shared_12_test_SOURCES = two_file_test_main.cc
 two_file_separate_shared_12_test_DEPENDENCIES = \
 	gcctestdir/ld two_file_shared_1.so two_file_shared_2.so
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 2879fc0..f0ae889 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -58,6 +58,7 @@ check_PROGRAMS = object_unittest$(EXEEXT) $(am__EXEEXT_1) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared_1_pic_2_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared_2_pic_1_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_same_shared_test \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_shared_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_12_test \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_21_test
 @GCC_FALSE@constructor_test_DEPENDENCIES = libgoldtest.a ../libgold.a \
@@ -217,6 +218,7 @@ libgoldtest_a_OBJECTS = $(am_libgoldtest_a_OBJECTS)
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared_1_pic_2_test$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_shared_2_pic_1_test$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_same_shared_test$(EXEEXT) \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_mixed_shared_test$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_12_test$(EXEEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_separate_shared_21_test$(EXEEXT)
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@am__EXEEXT_2 = two_file_shared_1_nonpic_test$(EXEEXT) \
@@ -399,6 +401,11 @@ am__tls_test_SOURCES_DIST = tls_test.cc tls_test_file2.cc \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@TLS_TRUE@	tls_test_file2.$(OBJEXT) \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@@TLS_TRUE@	tls_test_main.$(OBJEXT)
 tls_test_OBJECTS = $(am_tls_test_OBJECTS)
+am__two_file_mixed_shared_test_SOURCES_DIST = two_file_test_main.cc
+@GCC_TRUE@@NATIVE_LINKER_TRUE@am_two_file_mixed_shared_test_OBJECTS =  \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_test_main.$(OBJEXT)
+two_file_mixed_shared_test_OBJECTS =  \
+	$(am_two_file_mixed_shared_test_OBJECTS)
 am__two_file_pic_test_SOURCES_DIST = two_file_test_main.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@am_two_file_pic_test_OBJECTS =  \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	two_file_test_main.$(OBJEXT)
@@ -523,7 +530,8 @@ SOURCES = $(libgoldtest_a_SOURCES) basic_pic_test.c \
 	$(tls_pic_test_SOURCES) $(tls_shared_ie_test_SOURCES) \
 	$(tls_shared_nonpic_test_SOURCES) $(tls_shared_test_SOURCES) \
 	$(tls_static_pic_test_SOURCES) $(tls_static_test_SOURCES) \
-	$(tls_test_SOURCES) $(two_file_pic_test_SOURCES) \
+	$(tls_test_SOURCES) $(two_file_mixed_shared_test_SOURCES) \
+	$(two_file_pic_test_SOURCES) \
 	$(two_file_same_shared_nonpic_test_SOURCES) \
 	$(two_file_same_shared_test_SOURCES) \
 	$(two_file_separate_shared_12_nonpic_test_SOURCES) \
@@ -559,6 +567,7 @@ DIST_SOURCES = $(libgoldtest_a_SOURCES) basic_pic_test.c \
 	$(am__tls_static_pic_test_SOURCES_DIST) \
 	$(am__tls_static_test_SOURCES_DIST) \
 	$(am__tls_test_SOURCES_DIST) \
+	$(am__two_file_mixed_shared_test_SOURCES_DIST) \
 	$(am__two_file_pic_test_SOURCES_DIST) \
 	$(am__two_file_same_shared_nonpic_test_SOURCES_DIST) \
 	$(am__two_file_same_shared_test_SOURCES_DIST) \
@@ -796,6 +805,10 @@ object_unittest_SOURCES = object_unittest.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_same_shared_test_DEPENDENCIES = gcctestdir/ld two_file_shared.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_same_shared_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_same_shared_test_LDADD = two_file_shared.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_mixed_shared_test_SOURCES = two_file_test_main.cc
+@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_mixed_shared_test_DEPENDENCIES = gcctestdir/ld two_file_shared_mixed.so
+@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_mixed_shared_test_LDFLAGS = -Bgcctestdir/ -Wl,-R,.
+@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_mixed_shared_test_LDADD = two_file_shared_mixed.so
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_separate_shared_12_test_SOURCES = two_file_test_main.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_separate_shared_12_test_DEPENDENCIES = \
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	gcctestdir/ld two_file_shared_1.so two_file_shared_2.so
@@ -1066,6 +1079,9 @@ tls_static_test$(EXEEXT): $(tls_static_test_OBJECTS) $(tls_static_test_DEPENDENC
 tls_test$(EXEEXT): $(tls_test_OBJECTS) $(tls_test_DEPENDENCIES) 
 	@rm -f tls_test$(EXEEXT)
 	$(CXXLINK) $(tls_test_LDFLAGS) $(tls_test_OBJECTS) $(tls_test_LDADD) $(LIBS)
+two_file_mixed_shared_test$(EXEEXT): $(two_file_mixed_shared_test_OBJECTS) $(two_file_mixed_shared_test_DEPENDENCIES) 
+	@rm -f two_file_mixed_shared_test$(EXEEXT)
+	$(CXXLINK) $(two_file_mixed_shared_test_LDFLAGS) $(two_file_mixed_shared_test_OBJECTS) $(two_file_mixed_shared_test_LDADD) $(LIBS)
 two_file_pic_test$(EXEEXT): $(two_file_pic_test_OBJECTS) $(two_file_pic_test_DEPENDENCIES) 
 	@rm -f two_file_pic_test$(EXEEXT)
 	$(CXXLINK) $(two_file_pic_test_LDFLAGS) $(two_file_pic_test_OBJECTS) $(two_file_pic_test_LDADD) $(LIBS)
@@ -1467,6 +1483,8 @@ uninstall-am: uninstall-info-am
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_2_pic.o
 @GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared.so: two_file_test_1_pic.o two_file_test_2_pic.o gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_2_pic.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared_mixed.so: two_file_test_1_pic.o two_file_test_2.o gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1_pic.o two_file_test_2.o
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared_1_nonpic.so: two_file_test_1.o gcctestdir/ld
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@	$(CXXLINK) -Bgcctestdir/ -shared two_file_test_1.o
 @FN_PTRS_IN_SO_WITHOUT_PIC_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@two_file_shared_2_nonpic.so: two_file_test_2.o gcctestdir/ld
diff --git a/gold/testsuite/two_file_test.h b/gold/testsuite/two_file_test.h
index 63c529e..05e5cbe 100644
--- a/gold/testsuite/two_file_test.h
+++ b/gold/testsuite/two_file_test.h
@@ -24,6 +24,7 @@
 // file.  See two_file_test_1.cc for details.
 
 extern bool t1();
+extern bool t1a();
 extern int t1_2();
 
 extern bool t2();
diff --git a/gold/testsuite/two_file_test_2.cc b/gold/testsuite/two_file_test_2.cc
index 806409e..36873cc 100644
--- a/gold/testsuite/two_file_test_2.cc
+++ b/gold/testsuite/two_file_test_2.cc
@@ -33,6 +33,12 @@ t1_2()
   return 123;
 }
 
+bool
+t1a()
+{
+  return t1_2() == 123;
+}
+
 // 2  Code in file 1 refers to global data in file 2.
 
 int v2 = 456;
diff --git a/gold/testsuite/two_file_test_main.cc b/gold/testsuite/two_file_test_main.cc
index cd532ab..5dcf94b 100644
--- a/gold/testsuite/two_file_test_main.cc
+++ b/gold/testsuite/two_file_test_main.cc
@@ -36,6 +36,7 @@ main()
     v5[i] = v4[i];
 
   assert(t1());
+  assert(t1a());
   assert(t2());
   assert(t3());
   assert(t4());
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index afa8070..0b791f2 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -1032,7 +1032,7 @@ Target_x86_64::Scan::global(const General_options& options,
               gsym->set_needs_dynsym_value();
           }
         // Make a dynamic relocation if necessary.
-        if (gsym->needs_dynamic_reloc(true, false))
+        if (gsym->needs_dynamic_reloc(Symbol::ABSOLUTE_REF))
           {
             if (target->may_need_copy_reloc(gsym))
               {
@@ -1068,8 +1068,10 @@ Target_x86_64::Scan::global(const General_options& options,
         if (gsym->needs_plt_entry())
           target->make_plt_entry(symtab, layout, gsym);
         // Make a dynamic relocation if necessary.
-        bool is_function_call = (gsym->type() == elfcpp::STT_FUNC);
-        if (gsym->needs_dynamic_reloc(false, is_function_call))
+        int flags = Symbol::NON_PIC_REF;
+        if (gsym->type() == elfcpp::STT_FUNC)
+          flags |= Symbol::FUNCTION_CALL;
+        if (gsym->needs_dynamic_reloc(flags))
           {
             if (target->may_need_copy_reloc(gsym))
               {