gold patch committed: Adjust protected symbol function address

gold patch committed: Adjust protected symbol function address

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

The glibc testsuite has a test case which checks that when the main
executable refers to a protected function defined in a shared library,
the function address seen in the main executable is the same as the
function address seen in the shared library.  I'm not sure this is a
worthwhile test, because it fails when using gcc's visibility attribute.
It only works when using .protected in an asm statement.  When using the
visibility attribute, gcc refers to the symbol directly using a
PC-relative reloc when possible, rather than creating a GOT entry.
However, in order to pass the glibc testsuite, this patch implements
this functionality in gold.  Committed to mainline.

Ian


2011-07-12  Ian Lance Taylor  <iant@google.com>

	PR gold/12980
	* i386.cc (Target_i386::Scan::global): For a GOT reloc, use a
	GLOB_DAT relocation rather than a RELATIVE relocation for a
	protected symbol when creating a shared library.
	* x86_64.cc (Target_x86_64::Scan::global): Likewise.
	* testsuite/protected_1.cc (f2, get_f2_addr): New functions.
	* testsuite/protected_main_1.cc (main): Test that protected
	function has same address.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4527 bytes --]

Index: i386.cc
===================================================================
RCS file: /cvs/src/src/gold/i386.cc,v
retrieving revision 1.138
diff -u -p -r1.138 i386.cc
--- i386.cc	9 Jul 2011 00:47:11 -0000	1.138
+++ i386.cc	12 Jul 2011 22:28:32 -0000
@@ -1985,9 +1985,24 @@ Target_i386::Scan::global(Symbol_table* 
             // If this symbol is not fully resolved, we need to add a
             // GOT entry with a dynamic relocation.
             Reloc_section* rel_dyn = target->rel_dyn_section(layout);
+
+	    // Use a GLOB_DAT rather than a RELATIVE reloc if:
+	    //
+	    // 1) The symbol may be defined in some other module.
+	    //
+	    // 2) We are building a shared library and this is a
+	    // protected symbol; using GLOB_DAT means that the dynamic
+	    // linker can use the address of the PLT in the main
+	    // executable when appropriate so that function address
+	    // comparisons work.
+	    //
+	    // 3) This is a STT_GNU_IFUNC symbol in position dependent
+	    // code, again so that function address comparisons work.
             if (gsym->is_from_dynobj()
                 || gsym->is_undefined()
                 || gsym->is_preemptible()
+		|| (gsym->visibility() == elfcpp::STV_PROTECTED
+		    && parameters->options().shared())
 		|| (gsym->type() == elfcpp::STT_GNU_IFUNC
 		    && parameters->options().output_is_position_independent()))
               got->add_global_with_rel(gsym, GOT_TYPE_STANDARD,
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.136
diff -u -p -r1.136 x86_64.cc
--- x86_64.cc	9 Jul 2011 00:47:12 -0000	1.136
+++ x86_64.cc	12 Jul 2011 22:28:32 -0000
@@ -2436,9 +2436,24 @@ Target_x86_64::Scan::global(Symbol_table
             // If this symbol is not fully resolved, we need to add a
             // dynamic relocation for it.
             Reloc_section* rela_dyn = target->rela_dyn_section(layout);
+
+	    // Use a GLOB_DAT rather than a RELATIVE reloc if:
+	    //
+	    // 1) The symbol may be defined in some other module.
+	    //
+	    // 2) We are building a shared library and this is a
+	    // protected symbol; using GLOB_DAT means that the dynamic
+	    // linker can use the address of the PLT in the main
+	    // executable when appropriate so that function address
+	    // comparisons work.
+	    //
+	    // 3) This is a STT_GNU_IFUNC symbol in position dependent
+	    // code, again so that function address comparisons work.
 	    if (gsym->is_from_dynobj()
 		|| gsym->is_undefined()
 		|| gsym->is_preemptible()
+		|| (gsym->visibility() == elfcpp::STV_PROTECTED
+		    && parameters->options().shared())
 		|| (gsym->type() == elfcpp::STT_GNU_IFUNC
 		    && parameters->options().output_is_position_independent()))
               got->add_global_with_rela(gsym, GOT_TYPE_STANDARD, rela_dyn,
Index: testsuite/protected_1.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/protected_1.cc,v
retrieving revision 1.1
diff -u -p -r1.1 protected_1.cc
--- testsuite/protected_1.cc	6 May 2008 22:24:26 -0000	1.1
+++ testsuite/protected_1.cc	12 Jul 2011 22:28:32 -0000
@@ -31,3 +31,28 @@ f1()
 {
   return 1;
 }
+
+// The function f2 is used to test that the executable can see the
+// same function address for a protected function in the executable
+// and in the shared library.  We can't use the visibility attribute
+// here, becaues that may cause gcc to generate a PC relative reloc;
+// we need it to get the value from the GOT.  I'm not sure this is
+// really useful, given that it doesn't work with the visibility
+// attribute.  This test exists here mainly because the glibc
+// testsuite has the same test, and we want to make sure that gold
+// passes the glibc testsuite.
+
+extern "C" int f2();
+asm(".protected f2");
+
+extern "C" int
+f2()
+{
+  return 2;
+}
+
+int
+(*get_f2_addr())()
+{
+  return f2;
+}
Index: testsuite/protected_main_1.cc
===================================================================
RCS file: /cvs/src/src/gold/testsuite/protected_main_1.cc,v
retrieving revision 1.1
diff -u -p -r1.1 protected_main_1.cc
--- testsuite/protected_main_1.cc	6 May 2008 22:24:26 -0000	1.1
+++ testsuite/protected_main_1.cc	12 Jul 2011 22:28:32 -0000
@@ -28,9 +28,13 @@
 extern bool t1();
 extern bool t2();
 
+extern "C" int f2();
+extern int (*get_f2_addr()) ();
+
 int
 main()
 {
   assert(t1());
   assert(t2());
+  assert(&f2 == get_f2_addr());
 }

Re: gold patch committed: Adjust protected symbol function address

[H.J. Lu]

On Tue, Jul 12, 2011 at 3:32 PM, Ian Lance Taylor <iant@google.com> wrote:
> The glibc testsuite has a test case which checks that when the main
> executable refers to a protected function defined in a shared library,
> the function address seen in the main executable is the same as the
> function address seen in the shared library.  I'm not sure this is a
> worthwhile test, because it fails when using gcc's visibility attribute.

See

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19520
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35513


-- 
H.J.