This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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: RFC: PATCHES: Properly handle reference to protected data on x86

From: "H.J. Lu" <hjl dot tools at gmail dot com>
To: GCC Patches <gcc-patches at gcc dot gnu dot org>, GNU C Library <libc-alpha at sourceware dot org>, Binutils <binutils at sourceware dot org>
Date: Thu, 5 Mar 2015 06:39:10 -0800
Subject: Re: RFC: PATCHES: Properly handle reference to protected data on x86
Authentication-results: sourceware.org; auth=none

On Wed, Mar 4, 2015 at 3:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Protected symbol means that it can't be pre-emptied.  It
> doesn't mean its address won't be external.  This is true
> for pointer to protected function.  With copy relocation,
> address of protected data defined in the shared library may
> also be external.  We only know that for sure at run-time.
> Here are patches for glibc, binutils and GCC to handle it
> properly.
>
> Any comments?

This is the binutils patch I checked in.  It basically reverted
the change for

https://sourceware.org/bugzilla/show_bug.cgi?id=15228

on x86.  Copy relocations against protected symbols should
work.

-- 
H.J.
---
bfd/

PR ld/pr15228
PR ld/pr17709
* elf-bfd.h (elf_backend_data): Add extern_protected_data.
* elf32-i386.c (elf_backend_extern_protected_data): New.
Defined to 1.
* elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
* elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
copy relocs against protected symbols if extern_protected_data
is true.
(_bfd_elf_symbol_refs_local_p): Don't return true on protected
non-function symbols if extern_protected_data is true.
* elfxx-target.h (elf_backend_extern_protected_data): New.
Default to 0.
(elfNN_bed): Initialize extern_protected_data with
elf_backend_extern_protected_data.

ld/testsuite/

PR ld/pr15228
PR ld/pr17709
* ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
* ld-i386/pr17709-nacl.rd: New file.
* ld-i386/pr17709.rd: Likewise.
* ld-i386/pr17709a.s: Likewise.
* ld-i386/pr17709b.s: Likewise.
* ld-i386/protected3.d: Updated.
* ld-i386/protected3.s: Likewise.
* ld-x86-64/pr17709-nacl.rd: New file.
* ld-x86-64/pr17709.rd: Likewise.
* ld-x86-64/pr17709a.s: Likewise.
* ld-x86-64/pr17709b.s: Likewise.
* ld-x86-64/protected3.d: Updated.
* ld-x86-64/protected3.s: Likewise.
* ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.

From ca3fe95e469b9daec153caa2c90665f5daaec2b5 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 5 Mar 2015 06:34:39 -0800
Subject: [PATCH] Add extern_protected_data and set it for x86

With copy relocation, address of protected data defined in the shared
library may be external.  This patch adds extern_protected_data and
changes _bfd_elf_symbol_refs_local_p to return false for protected data
if extern_protected_data is true.

bfd/

	PR ld/pr15228
	PR ld/pr17709
	* elf-bfd.h (elf_backend_data): Add extern_protected_data.
	* elf32-i386.c (elf_backend_extern_protected_data): New.
	Defined to 1.
	* elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
	* elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
	copy relocs against protected symbols if extern_protected_data
	is true.
	(_bfd_elf_symbol_refs_local_p): Don't return true on protected
	non-function symbols if extern_protected_data is true.
	* elfxx-target.h (elf_backend_extern_protected_data): New.
	Default to 0.
	(elfNN_bed): Initialize extern_protected_data with
	elf_backend_extern_protected_data.

ld/testsuite/

	PR ld/pr15228
	PR ld/pr17709
	* ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
	* ld-i386/pr17709-nacl.rd: New file.
	* ld-i386/pr17709.rd: Likewise.
	* ld-i386/pr17709a.s: Likewise.
	* ld-i386/pr17709b.s: Likewise.
	* ld-i386/protected3.d: Updated.
	* ld-i386/protected3.s: Likewise.
	* ld-x86-64/pr17709-nacl.rd: New file.
	* ld-x86-64/pr17709.rd: Likewise.
	* ld-x86-64/pr17709a.s: Likewise.
	* ld-x86-64/pr17709b.s: Likewise.
	* ld-x86-64/protected3.d: Updated.
	* ld-x86-64/protected3.s: Likewise.
	* ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.
---
 bfd/ChangeLog                          | 18 ++++++++++++++++++
 bfd/elf-bfd.h                          |  4 ++++
 bfd/elf32-i386.c                       |  1 +
 bfd/elf64-x86-64.c                     |  1 +
 bfd/elflink.c                          |  9 ++++++---
 bfd/elfxx-target.h                     |  6 +++++-
 ld/testsuite/ChangeLog                 | 19 +++++++++++++++++++
 ld/testsuite/ld-i386/i386.exp          |  4 ++++
 ld/testsuite/ld-i386/pr17709-nacl.rd   |  4 ++++
 ld/testsuite/ld-i386/pr17709.rd        |  4 ++++
 ld/testsuite/ld-i386/pr17709a.s        |  8 ++++++++
 ld/testsuite/ld-i386/pr17709b.s        |  5 +++++
 ld/testsuite/ld-i386/protected3.d      |  3 ++-
 ld/testsuite/ld-i386/protected3.s      |  3 ++-
 ld/testsuite/ld-x86-64/pr17709-nacl.rd |  4 ++++
 ld/testsuite/ld-x86-64/pr17709.rd      |  4 ++++
 ld/testsuite/ld-x86-64/pr17709a.s      |  8 ++++++++
 ld/testsuite/ld-x86-64/pr17709b.s      |  5 +++++
 ld/testsuite/ld-x86-64/protected3.d    |  3 ++-
 ld/testsuite/ld-x86-64/protected3.s    |  3 ++-
 ld/testsuite/ld-x86-64/x86-64.exp      |  4 ++++
 21 files changed, 112 insertions(+), 8 deletions(-)
 create mode 100644 ld/testsuite/ld-i386/pr17709-nacl.rd
 create mode 100644 ld/testsuite/ld-i386/pr17709.rd
 create mode 100644 ld/testsuite/ld-i386/pr17709a.s
 create mode 100644 ld/testsuite/ld-i386/pr17709b.s
 create mode 100644 ld/testsuite/ld-x86-64/pr17709-nacl.rd
 create mode 100644 ld/testsuite/ld-x86-64/pr17709.rd
 create mode 100644 ld/testsuite/ld-x86-64/pr17709a.s
 create mode 100644 ld/testsuite/ld-x86-64/pr17709b.s

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 3f8cc86..f29dec5 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,21 @@
+2015-03-05  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/pr15228
+	PR ld/pr17709
+	* elf-bfd.h (elf_backend_data): Add extern_protected_data.
+	* elf32-i386.c (elf_backend_extern_protected_data): New.
+	Defined to 1.
+	* elf64-x86-64.c (elf_backend_extern_protected_data): Likewise.
+	* elflink.c (_bfd_elf_adjust_dynamic_copy): Don't error on
+	copy relocs against protected symbols if extern_protected_data
+	is true.
+	(_bfd_elf_symbol_refs_local_p): Don't return true on protected
+	non-function symbols if extern_protected_data is true.
+	* elfxx-target.h (elf_backend_extern_protected_data): New.
+	Default to 0.
+	(elfNN_bed): Initialize extern_protected_data with
+	elf_backend_extern_protected_data.
+
 2015-03-05  Nick Clifton  <nickc@redhat.com>
 
 	PR binutils/18025
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 156eec7..13c32e0 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1359,6 +1359,10 @@ struct elf_backend_data
      in length rather than sec->size in length, if sec->rawsize is
      non-zero and smaller than sec->size.  */
   unsigned caches_rawsize : 1;
+
+  /* Address of protected data defined in the shared library may be
+     external, i.e., due to copy relocation.   */
+  unsigned extern_protected_data : 1;
 };
 
 /* Information about reloc sections associated with a bfd_elf_section_data
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 3f16fc1..52f4d33 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -5292,6 +5292,7 @@ elf_i386_add_symbol_hook (bfd * abfd,
 #define elf_backend_want_plt_sym	0
 #define elf_backend_got_header_size	12
 #define elf_backend_plt_alignment	4
+#define elf_backend_extern_protected_data 1
 
 /* Support RELA for objdump of prelink objects.  */
 #define elf_info_to_howto		      elf_i386_info_to_howto_rel
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index a4974ce..74d1d06 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -5868,6 +5868,7 @@ static const struct bfd_elf_special_section
 #define elf_backend_got_header_size	    (GOT_ENTRY_SIZE*3)
 #define elf_backend_rela_normal		    1
 #define elf_backend_plt_alignment           4
+#define elf_backend_extern_protected_data   1
 
 #define elf_info_to_howto		    elf_x86_64_info_to_howto
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index ec1e4df..6ee6499 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2671,7 +2671,9 @@ _bfd_elf_adjust_dynamic_copy (struct bfd_link_info *info,
   /* Increment the size of DYNBSS to make room for the symbol.  */
   dynbss->size += h->size;
 
-  if (h->protected_def)
+  /* No error if extern_protected_data is true.  */
+  if (h->protected_def
+      && !get_elf_backend_data (dynbss->owner)->extern_protected_data)
     {
       info->callbacks->einfo
 	(_("%P: copy reloc against protected `%T' is invalid\n"),
@@ -2835,8 +2837,9 @@ _bfd_elf_symbol_refs_local_p (struct elf_link_hash_entry *h,
 
   bed = get_elf_backend_data (hash_table->dynobj);
 
-  /* STV_PROTECTED non-function symbols are local.  */
-  if (!bed->is_function_type (h->type))
+  /* If extern_protected_data is false, STV_PROTECTED non-function
+     symbols are local.  */
+  if (!bed->extern_protected_data && !bed->is_function_type (h->type))
     return TRUE;
 
   /* Function pointer equality tests may require that STV_PROTECTED
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 211c0a1..9760db4 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -117,6 +117,9 @@
 #ifndef elf_backend_caches_rawsize
 #define elf_backend_caches_rawsize 0
 #endif
+#ifndef elf_backend_extern_protected_data
+#define elf_backend_extern_protected_data 0
+#endif
 #ifndef elf_backend_stack_align
 #define elf_backend_stack_align 16
 #endif
@@ -801,7 +804,8 @@ static struct elf_backend_data elfNN_bed =
   elf_backend_want_dynbss,
   elf_backend_want_p_paddr_set_to_zero,
   elf_backend_default_execstack,
-  elf_backend_caches_rawsize
+  elf_backend_caches_rawsize,
+  elf_backend_extern_protected_data
 };
 
 /* Forward declaration for use when initialising alternative_target field.  */
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index acd6d81..b7defad 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,22 @@
+2015-03-05  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/pr15228
+	PR ld/pr17709
+	* ld-i386/i386.exp (i386tests): Add a test for PR ld/17709.
+	* ld-i386/pr17709-nacl.rd: New file.
+	* ld-i386/pr17709.rd: Likewise.
+	* ld-i386/pr17709a.s: Likewise.
+	* ld-i386/pr17709b.s: Likewise.
+	* ld-i386/protected3.d: Updated.
+	* ld-i386/protected3.s: Likewise.
+	* ld-x86-64/pr17709-nacl.rd: New file.
+	* ld-x86-64/pr17709.rd: Likewise.
+	* ld-x86-64/pr17709a.s: Likewise.
+	* ld-x86-64/pr17709b.s: Likewise.
+	* ld-x86-64/protected3.d: Updated.
+	* ld-x86-64/protected3.s: Likewise.
+	* ld-x86-64/x86-64.exp (x86_64tests): Add a test for PR ld/17709.
+
 2015-03-04  Richard Sandiford  <richard.sandiford@arm.com>
 
 	PR gas/17843
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 4fc2359..3fdd39c 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -191,6 +191,10 @@ set i386tests {
      "--32" {pr17306b.s} {} ""}
     {"PR ld/17306 (2)" "-melf_i386 -shared -Bsymbolic --just-symbols=tmpdir/pr17306b.o" ""
      "--32" {pr17306a.s} {} "libpr17306.so"}
+    {"PR ld/17709 (1)" "-melf_i386 -shared" ""
+     "--32" {pr17709a.s} {} "libpr17709.so"}
+    {"PR ld/17709 (2)" "-melf_i386 tmpdir/libpr17709.so" ""
+     "--32" {pr17709b.s} {{readelf -r pr17709.rd}} "pr17709"}
 }
 
 # So as to avoid rewriting every last test case here in a nacl variant,
diff --git a/ld/testsuite/ld-i386/pr17709-nacl.rd b/ld/testsuite/ld-i386/pr17709-nacl.rd
new file mode 100644
index 0000000..ac9d174
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr17709-nacl.rd
@@ -0,0 +1,4 @@
+
+Relocation section '.rel\..*' at offset .* contains 1 entries:
+ Offset     Info    Type            Sym\.Value  Sym\. Name
+[0-9a-f ]+R_386_COPY +[0-9a-f]+ +foo
diff --git a/ld/testsuite/ld-i386/pr17709.rd b/ld/testsuite/ld-i386/pr17709.rd
new file mode 100644
index 0000000..ac9d174
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr17709.rd
@@ -0,0 +1,4 @@
+
+Relocation section '.rel\..*' at offset .* contains 1 entries:
+ Offset     Info    Type            Sym\.Value  Sym\. Name
+[0-9a-f ]+R_386_COPY +[0-9a-f]+ +foo
diff --git a/ld/testsuite/ld-i386/pr17709a.s b/ld/testsuite/ld-i386/pr17709a.s
new file mode 100644
index 0000000..8750f5e
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr17709a.s
@@ -0,0 +1,8 @@
+	.protected	foo
+.globl foo
+	.data
+	.align 4
+	.type	foo, @object
+	.size	foo, 4
+foo:
+	.long	1
diff --git a/ld/testsuite/ld-i386/pr17709b.s b/ld/testsuite/ld-i386/pr17709b.s
new file mode 100644
index 0000000..c57f404
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr17709b.s
@@ -0,0 +1,5 @@
+	.text
+	.globl	_start
+	.type	_start, @function
+_start:
+	movl	foo, %eax
diff --git a/ld/testsuite/ld-i386/protected3.d b/ld/testsuite/ld-i386/protected3.d
index aafa2d8..47ab4e1 100644
--- a/ld/testsuite/ld-i386/protected3.d
+++ b/ld/testsuite/ld-i386/protected3.d
@@ -8,6 +8,7 @@
 Disassembly of section .text:
 
 0+[a-f0-9]+ <bar>:
-[ 	]*[a-f0-9]+:	8b 81 [a-f0-9][a-f0-9] [a-f0-9][a-f0-9] 00 00    	mov    0x[a-f0-9]+\(%ecx\),%eax
+[ 	]*[a-f0-9]+:	8b 81 [a-f0-9][a-f0-9] [a-f0-9][a-f0-9] ff ff    	mov    -0x[a-f0-9]+\(%ecx\),%eax
+[ 	]*[a-f0-9]+:	8b 00                	mov    \(%eax\),%eax
 [ 	]*[a-f0-9]+:	c3                   	ret    
 #pass
diff --git a/ld/testsuite/ld-i386/protected3.s b/ld/testsuite/ld-i386/protected3.s
index 7a605a2..4dd2115 100644
--- a/ld/testsuite/ld-i386/protected3.s
+++ b/ld/testsuite/ld-i386/protected3.s
@@ -10,6 +10,7 @@ foo:
 .globl bar
 	.type	bar, @function
 bar:
-	movl	foo@GOTOFF(%ecx), %eax
+	movl	foo@GOT(%ecx), %eax
+	movl	(%eax), %eax
 	ret
 	.size	bar, .-bar
diff --git a/ld/testsuite/ld-x86-64/pr17709-nacl.rd b/ld/testsuite/ld-x86-64/pr17709-nacl.rd
new file mode 100644
index 0000000..f1baa38
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr17709-nacl.rd
@@ -0,0 +1,4 @@
+
+Relocation section '.rela\..*' at offset .* contains 1 entries:
+ +Offset +Info +Type +Symbol's Value +Symbol's Name \+ Addend
+[0-9a-f ]+R_X86_64_COPY+[0-9a-f ]+ +foo \+ 0
diff --git a/ld/testsuite/ld-x86-64/pr17709.rd b/ld/testsuite/ld-x86-64/pr17709.rd
new file mode 100644
index 0000000..f1baa38
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr17709.rd
@@ -0,0 +1,4 @@
+
+Relocation section '.rela\..*' at offset .* contains 1 entries:
+ +Offset +Info +Type +Symbol's Value +Symbol's Name \+ Addend
+[0-9a-f ]+R_X86_64_COPY+[0-9a-f ]+ +foo \+ 0
diff --git a/ld/testsuite/ld-x86-64/pr17709a.s b/ld/testsuite/ld-x86-64/pr17709a.s
new file mode 100644
index 0000000..8750f5e
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr17709a.s
@@ -0,0 +1,8 @@
+	.protected	foo
+.globl foo
+	.data
+	.align 4
+	.type	foo, @object
+	.size	foo, 4
+foo:
+	.long	1
diff --git a/ld/testsuite/ld-x86-64/pr17709b.s b/ld/testsuite/ld-x86-64/pr17709b.s
new file mode 100644
index 0000000..c57f404
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr17709b.s
@@ -0,0 +1,5 @@
+	.text
+	.globl	_start
+	.type	_start, @function
+_start:
+	movl	foo, %eax
diff --git a/ld/testsuite/ld-x86-64/protected3.d b/ld/testsuite/ld-x86-64/protected3.d
index 22a36ac..d8f09da 100644
--- a/ld/testsuite/ld-x86-64/protected3.d
+++ b/ld/testsuite/ld-x86-64/protected3.d
@@ -8,6 +8,7 @@
 Disassembly of section .text:
 
 0+[a-f0-9]+ <bar>:
-[ 	]*[a-f0-9]+:	8b 05 ([0-9a-f]{2} ){4} *	mov    0x[a-f0-9]+\(%rip\),%eax        # [a-f0-9]+ <foo>
+[ 	]*[a-f0-9]+:	48 8b 05 ([0-9a-f]{2} ){4} *	mov    0x[a-f0-9]+\(%rip\),%rax        # [a-f0-9]+ <_DYNAMIC\+0x[a-f0-9]+>
+[ 	]*[a-f0-9]+:	8b 00                	mov    \(%rax\),%eax
 [ 	]*[a-f0-9]+:	c3                   	retq *
 #pass
diff --git a/ld/testsuite/ld-x86-64/protected3.s b/ld/testsuite/ld-x86-64/protected3.s
index e4af6e7..7538050 100644
--- a/ld/testsuite/ld-x86-64/protected3.s
+++ b/ld/testsuite/ld-x86-64/protected3.s
@@ -10,6 +10,7 @@ foo:
 .globl bar
 	.type	bar, @function
 bar:
-	movl	foo(%rip), %eax
+	movq	foo@GOTPCREL(%rip), %rax
+	movl	(%rax), %eax
 	ret
 	.size	bar, .-bar
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 9bb08bd..43427be 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -144,6 +144,10 @@ set x86_64tests {
      "--64" {pr17306b.s} {} ""}
     {"PR ld/17306 (2)" "-melf_x86_64 -shared -Bsymbolic --just-symbols=tmpdir/pr17306b.o" ""
      "--64" {pr17306a.s} {} "libpr17306.so"}
+    {"PR ld/17709 (1)" "-melf_x86_64 -shared" ""
+     "--64" {pr17709a.s} {} "libpr17709.so"}
+    {"PR ld/17709 (2)" "-melf_x86_64 tmpdir/libpr17709.so" ""
+     "--64" {pr17709b.s} {{readelf -rW pr17709.rd}} "pr17709"}
 }
 
 # So as to avoid rewriting every last test case here in a nacl variant,
-- 
2.1.0

Follow-Ups:
- Re: RFC: PATCHES: Properly handle reference to protected data on x86
  - From: Rich Felker
- Re: RFC: PATCHES: Properly handle reference to protected data on x86
  - From: H.J. Lu

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