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]

gold patch committed: Support LD_PRELOAD defining weak undefined symbol


Paul Pluzhnikov ran across a case where gold did not work correctly.
If a program calls a weak function which was not defined when the
program was linked, but was defined at runtime via LD_PRELOAD, then
gold would not call the function as it should.  This only happened
when the program code was compiled with -fPIC.  The bug was that the
function call relocation would be resolved against the symbol's value
(i.e., 0, since the symbol was not defined) rather than against the
PLT entry for the symbol.

I committed this patch to fix the problem, along with a test case.  I
consolidated the logic for when to use the PLT entry into a single
Symbol function.  This fixed some cases where we arguably did the
wrong thing for certain relocations for the x86_64, sparc, and powerpc
targets.  It fixes the case described above for all targets.

I also added a couple of missing dependencies in testsuite/Makefile.am
that I happened to notice while adding the test case.

Ian


2008-08-04  Ian Lance Taylor  <iant@google.com>

	* symtab.h (Symbol::use_plt_offset): New function.
	* i386.cc (Relocate::relocate): Call Symbol::use_plt_offset.
	* powerpc.cc (Relocate::relocate): Likewise.
	* sparc.cc (Relocate::relocate): Likewise.
	* x86_64.cc (Relocate::relocate): Likewise.
	* testsuite/weak_plt.sh: New test.
	* testsuite/weak_plt_main.cc: New test.
	* testsuite/weak_plt_shared.cc: New test.
	* testsuite/Makefile.am (check_SCRIPTS): Add weak_plt.sh.
	(check_PROGRAMS): Add weak_plt.
	(check_DATA): Add weak_plt_shared.so.
	(weak_plt_main_pic.o, weak_plt): New targets.
	(weak_plt_shared_pic.o, weak_plt_shared.so): New targets.
	* testsuite/Makefile.in: Rebuild.

	* testsuite/Makefile.am (weak_alias_test_1.so): Depend upon
	gcctestdir/ld.
	(weak_alias_test_2.so, weak_alias_test_4.so): Likewise.
	* testsuite/Makefile.in: Rebuild.


Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.78
diff -p -u -r1.78 i386.cc
--- i386.cc	10 Jul 2008 23:01:19 -0000	1.78
+++ i386.cc	4 Aug 2008 22:17:44 -0000
@@ -1595,15 +1595,10 @@ Target_i386::Relocate::relocate(const Re
 
   // 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->options().shared()
-              && (gsym->is_undefined() || gsym->is_preemptible())))
-      && gsym->has_plt_offset()
-      && (!is_nonpic || !parameters->options().shared()))
+      && gsym->use_plt_offset(r_type == elfcpp::R_386_PC8
+			      || r_type == elfcpp::R_386_PC16
+			      || r_type == elfcpp::R_386_PC32))
     {
       symval.set_output_value(target->plt_section()->address()
 			      + gsym->plt_offset());
Index: powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.5
diff -p -u -r1.5 powerpc.cc
--- powerpc.cc	10 Jul 2008 23:01:19 -0000	1.5
+++ powerpc.cc	4 Aug 2008 22:17:44 -0000
@@ -1547,10 +1547,12 @@ Target_powerpc<size, big_endian>::Reloca
   // Pick the value to use for symbols defined in shared objects.
   Symbol_value<size> symval;
   if (gsym != NULL
-      && (gsym->is_from_dynobj()
-          || (parameters->options().shared()
-              && (gsym->is_undefined() || gsym->is_preemptible())))
-      && gsym->has_plt_offset())
+      && gsym->use_plt_offset(r_type == elfcpp::R_POWERPC_REL24
+			      || r_type == elfcpp::R_PPC_LOCAL24PC
+			      || r_type == elfcpp::R_PPC_REL16
+			      || r_type == elfcpp::R_PPC_REL16_LO
+			      || r_type == elfcpp::R_PPC_REL16_HI
+			      || r_type == elfcpp::R_PPC_REL16_HA))
     {
       elfcpp::Elf_Xword value;
 
Index: sparc.cc
===================================================================
RCS file: /cvs/src/src/gold/sparc.cc,v
retrieving revision 1.9
diff -p -u -r1.9 sparc.cc
--- sparc.cc	10 Jul 2008 23:01:19 -0000	1.9
+++ sparc.cc	4 Aug 2008 22:17:44 -0000
@@ -2332,10 +2332,19 @@ Target_sparc<size, big_endian>::Relocate
   // Pick the value to use for symbols defined in shared objects.
   Symbol_value<size> symval;
   if (gsym != NULL
-      && (gsym->is_from_dynobj()
-          || (parameters->options().shared()
-              && (gsym->is_undefined() || gsym->is_preemptible())))
-      && gsym->has_plt_offset())
+      && gsym->use_plt_offset(r_type == elfcpp::R_SPARC_DISP8
+			      || r_type == elfcpp::R_SPARC_DISP16
+			      || r_type == elfcpp::R_SPARC_DISP32
+			      || r_type == elfcpp::R_SPARC_DISP64
+			      || r_type == elfcpp::R_SPARC_PC_HH22
+			      || r_type == elfcpp::R_SPARC_PC_HM10
+			      || r_type == elfcpp::R_SPARC_PC_LM22
+			      || r_type == elfcpp::R_SPARC_PC10
+			      || r_type == elfcpp::R_SPARC_PC22
+			      || r_type == elfcpp::R_SPARC_WDISP30
+			      || r_type == elfcpp::R_SPARC_WDISP22
+			      || r_type == elfcpp::R_SPARC_WDISP19
+			      || r_type == elfcpp::R_SPARC_WDISP16))
     {
       elfcpp::Elf_Xword value;
 
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.81
diff -p -u -r1.81 symtab.h
--- symtab.h	23 Jul 2008 14:36:09 -0000	1.81
+++ symtab.h	4 Aug 2008 22:17:44 -0000
@@ -577,6 +577,48 @@ class Symbol
     return false;
   }
 
+  // Whether we should use the PLT offset associated with a symbol for
+  // a relocation.  IS_NON_PIC_REFERENCE is true if this is a non-PIC
+  // reloc--the same set of relocs for which we would pass NON_PIC_REF
+  // to the needs_dynamic_reloc function.
+
+  bool
+  use_plt_offset(bool is_non_pic_reference) const
+  {
+    // If the symbol doesn't have a PLT offset, then naturally we
+    // don't want to use it.
+    if (!this->has_plt_offset())
+      return false;
+
+    // If we are going to generate a dynamic relocation, then we will
+    // wind up using that, so no need to use the PLT entry.
+    if (this->needs_dynamic_reloc(FUNCTION_CALL
+				  | (is_non_pic_reference
+				     ? NON_PIC_REF
+				     : 0)))
+      return false;
+
+    // If the symbol is from a dynamic object, we need to use the PLT
+    // entry.
+    if (this->is_from_dynobj())
+      return true;
+
+    // If we are generating a shared object, and this symbol is
+    // undefined or preemptible, we need to use the PLT entry.
+    if (parameters->options().shared()
+	&& (this->is_undefined() || this->is_preemptible()))
+      return true;
+
+    // If this is a weak undefined symbol, we need to use the PLT
+    // entry; the symbol may be defined by a library loaded at
+    // runtime.
+    if (this->is_weak_undefined())
+      return true;
+
+    // Otherwise we can use the regular definition.
+    return false;
+  }
+
   // Given a direct absolute static relocation against
   // the global symbol, where a dynamic relocation is needed, this
   // function returns whether a relative dynamic relocation can be used.
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.71
diff -p -u -r1.71 x86_64.cc
--- x86_64.cc	10 Jul 2008 23:01:20 -0000	1.71
+++ x86_64.cc	4 Aug 2008 22:17:45 -0000
@@ -1665,10 +1665,10 @@ Target_x86_64::Relocate::relocate(const 
   // Pick the value to use for symbols defined in shared objects.
   Symbol_value<64> symval;
   if (gsym != NULL
-      && (gsym->is_from_dynobj()
-          || (parameters->options().shared()
-              && (gsym->is_undefined() || gsym->is_preemptible())))
-      && gsym->has_plt_offset())
+      && gsym->use_plt_offset(r_type == elfcpp::R_X86_64_PC64
+			      || r_type == elfcpp::R_X86_64_PC32
+			      || r_type == elfcpp::R_X86_64_PC16
+			      || r_type == elfcpp::R_X86_64_PC8))
     {
       symval.set_output_value(target->plt_section()->address()
 			      + gsym->plt_offset());
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.73
diff -p -u -r1.73 Makefile.am
--- testsuite/Makefile.am	23 Jul 2008 23:44:02 -0000	1.73
+++ testsuite/Makefile.am	4 Aug 2008 22:17:45 -0000
@@ -415,19 +415,31 @@ weak_alias_test_LDADD = \
 	weak_alias_test_4.so
 weak_alias_test_1_pic.o: weak_alias_test_1.cc
 	$(CXXCOMPILE) -c -fpic -o $@ $<
-weak_alias_test_1.so: weak_alias_test_1_pic.o
+weak_alias_test_1.so: weak_alias_test_1_pic.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -shared weak_alias_test_1_pic.o
 weak_alias_test_2_pic.o: weak_alias_test_2.cc
 	$(CXXCOMPILE) -c -fpic -o $@ $<
-weak_alias_test_2.so: weak_alias_test_2_pic.o
+weak_alias_test_2.so: weak_alias_test_2_pic.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -shared weak_alias_test_2_pic.o
 weak_alias_test_3.o: weak_alias_test_3.cc
 	$(CXXCOMPILE) -c -o $@ $<
 weak_alias_test_4_pic.o: weak_alias_test_4.cc
 	$(CXXCOMPILE) -c -fpic -o $@ $<
-weak_alias_test_4.so: weak_alias_test_4_pic.o
+weak_alias_test_4.so: weak_alias_test_4_pic.o gcctestdir/ld
 	$(CXXLINK) -Bgcctestdir/ -shared weak_alias_test_4_pic.o
 
+check_SCRIPTS += weak_plt.sh
+check_PROGRAMS += weak_plt
+check_DATA += weak_plt_shared.so
+weak_plt_main_pic.o: weak_plt_main.cc
+	$(CXXCOMPILE) -c -fpic -o $@ $<
+weak_plt: weak_plt_main_pic.o gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ weak_plt_main_pic.o
+weak_plt_shared_pic.o: weak_plt_shared.cc
+	$(CXXCOMPILE) -c -fpic -o $@ $<
+weak_plt_shared.so: weak_plt_shared_pic.o gcctestdir/ld
+	$(CXXLINK) -Bgcctestdir/ -shared weak_plt_shared_pic.o
+
 check_PROGRAMS += copy_test
 copy_test_SOURCES = copy_test.cc
 copy_test_DEPENDENCIES = gcctestdir/ld copy_test_1.so copy_test_2.so
Index: testsuite/weak_plt.sh
===================================================================
RCS file: testsuite/weak_plt.sh
diff -N testsuite/weak_plt.sh
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/weak_plt.sh	4 Aug 2008 22:17:45 -0000
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# weak_plt.sh -- test calling a weak undefined function.
+
+# 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.
+
+# This tests a call to a weak undefined function.  We use LD_PRELOAD
+# to force the function to be defined.
+
+LD_PRELOAD=./weak_plt_shared.so ./weak_plt
Index: testsuite/weak_plt_main.cc
===================================================================
RCS file: testsuite/weak_plt_main.cc
diff -N testsuite/weak_plt_main.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/weak_plt_main.cc	4 Aug 2008 22:17:45 -0000
@@ -0,0 +1,33 @@
+// weak_plt_main.cc -- test call to weak undefined function 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.
+
+#include <cstdio>
+
+extern int weak_function() __attribute__((weak));
+
+int
+main()
+{
+  if (weak_function)
+    return weak_function();
+  return 0;
+}
Index: testsuite/weak_plt_shared.cc
===================================================================
RCS file: testsuite/weak_plt_shared.cc
diff -N testsuite/weak_plt_shared.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/weak_plt_shared.cc	4 Aug 2008 22:17:45 -0000
@@ -0,0 +1,29 @@
+// weak_plt_shared.cc -- test call to weak undefined function 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.
+
+#include <cstdio>
+
+int
+weak_function()
+{
+  return 0;
+}

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