[PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic

public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed

* [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic
@ 2015-04-25 21:46 H.J. Lu
  2015-04-25 23:46 ` Alan Modra
  0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2015-04-25 21:46 UTC (permalink / raw)
  To: binutils

If a global protected symbol is bound locally by -Bsymbolic, there is no
need to keep protected visibility which hich may have overhead at run-time.
OK for master branch?

Thanks.

H.J.
---
bfd/

	PR ld/18329
	* elflink.c (elf_link_output_extsym): Turn off protected
	visibility on locally bound symbol.

ld/testsuite/

	PR ld/18329
	* ld-elf/pr18329.d: New file.
	* ld-elf/pr18329.s: Likewise.
---
 bfd/elflink.c                 |  3 +++
 ld/testsuite/ld-elf/pr18329.d |  8 ++++++++
 ld/testsuite/ld-elf/pr18329.s | 11 +++++++++++
 3 files changed, 22 insertions(+)
 create mode 100644 ld/testsuite/ld-elf/pr18329.d
 create mode 100644 ld/testsuite/ld-elf/pr18329.s

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 6efe1e4..9cc5742 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -8973,6 +8973,9 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
     sym.st_info = ELF_ST_INFO (STB_WEAK, h->type);
   else
     sym.st_info = ELF_ST_INFO (STB_GLOBAL, h->type);
+  if (SYMBOLIC_BIND (flinfo->info, h))
+    /* Turn off protected visibility on locally bound symbol.  */
+    sym.st_other &= ~ELF_ST_VISIBILITY (STV_PROTECTED);
   sym.st_target_internal = h->target_internal;
 
   switch (h->root.type)
diff --git a/ld/testsuite/ld-elf/pr18329.d b/ld/testsuite/ld-elf/pr18329.d
new file mode 100644
index 0000000..6770847
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr18329.d
@@ -0,0 +1,8 @@
+#ld: -shared -Bsymbolic
+#readelf: --wide --dyn-syms
+#target: *-*-linux* *-*-gnu*
+#notarget: cris*-*-elf frv-*-*
+
+#...
+ +[0-9]+: +[0-9a-f]+ +[0-9]+ +OBJECT +GLOBAL +DEFAULT +[1-9]+ xxxxx
+#pass
diff --git a/ld/testsuite/ld-elf/pr18329.s b/ld/testsuite/ld-elf/pr18329.s
new file mode 100644
index 0000000..27fa2d0
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr18329.s
@@ -0,0 +1,11 @@
+	.text
+	.globl	bar
+	.type	bar, %function
+bar:
+	.dc.a xxxxx
+	.data
+	.globl xxxxx
+	.type   xxxxx, %object
+	.protected	xxxxx
+xxxxx:
+	.byte 0
-- 
2.1.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic
  2015-04-25 21:46 [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic H.J. Lu
@ 2015-04-25 23:46 ` Alan Modra
  2015-04-26  0:45   ` H.J. Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2015-04-25 23:46 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Sat, Apr 25, 2015 at 02:46:38PM -0700, H.J. Lu wrote:
> If a global protected symbol is bound locally by -Bsymbolic, there is no
> need to keep protected visibility which hich may have overhead at run-time.

For this change to be safe you'd need to show that all ELF targets
supporting dynamic objects also properly support -Bsymbolic for both
functions and variables.  ie. that none emit dynamic relocations for
the symbol when -Bsymbolic.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic
  2015-04-25 23:46 ` Alan Modra
@ 2015-04-26  0:45   ` H.J. Lu
  2015-04-26  1:48     ` Alan Modra
  0 siblings, 1 reply; 5+ messages in thread
From: H.J. Lu @ 2015-04-26  0:45 UTC (permalink / raw)
  To: binutils

On Sun, Apr 26, 2015 at 09:16:09AM +0930, Alan Modra wrote:
> On Sat, Apr 25, 2015 at 02:46:38PM -0700, H.J. Lu wrote:
> > If a global protected symbol is bound locally by -Bsymbolic, there is no
> > need to keep protected visibility which hich may have overhead at run-time.
> 
> For this change to be safe you'd need to show that all ELF targets
> supporting dynamic objects also properly support -Bsymbolic for both
> functions and variables.  ie. that none emit dynamic relocations for
> the symbol when -Bsymbolic.
> 

Good point.  Here is a patch to make it backend dependent.  I will
check it in shortly.

Thanks.


H.J.
---
From 73baa6fe89176d02aceac7c589fe873fa371237e Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 25 Apr 2015 14:42:50 -0700
Subject: [PATCH] Turn off protected visibility on locally bound symbol

If backend doesn't generate dynamic relocations against protected
functions and variables when they are bound locally there is no need
to keep protected visibility which may have overhead at run-time.

bfd/

	PR ld/18329
	* elf-bfd.h (elf_backend_data): Add no_dyn_reloc_with_symbolic.
	* elf32-i386.c (elf_backend_no_dyn_reloc_with_symbolic): New.
	Defined to 1.
	* elf64-x86-64.c (elf_backend_no_dyn_reloc_with_symbolic): Likewise.
	* elflink.c (elf_link_output_extsym): Turn off protected
	visibility on locally bound symbol if backend supports it.
	* elfxx-target.h (elf_backend_no_dyn_reloc_with_symbolic): New.
	Default to 0.
	(elfNN_bed): Initialize no_dyn_reloc_with_symbolic with
	elf_backend_no_dyn_reloc_with_symbolic.

ld/testsuite/

	PR ld/18329
	* ld-i386/i386.exp: Run pr18329a and pr18329b.
	* ld-x86-64/x86-64.exp: Likewise.
	* ld-i386/pr18329.s: New file.
	* ld-i386/pr18329a.d: Likewise.
	* ld-i386/pr18329b.d: Likewise.
	* ld-x86-64/pr18329.s: Likewise.
	* ld-x86-64/pr18329a.d: Likewise.
	* ld-x86-64/pr18329b.d: Likewise.
---
 bfd/elf-bfd.h                     |  4 ++++
 bfd/elf32-i386.c                  |  1 +
 bfd/elf64-x86-64.c                |  1 +
 bfd/elflink.c                     |  6 ++++++
 bfd/elfxx-target.h                |  6 +++++-
 ld/testsuite/ld-i386/i386.exp     |  2 ++
 ld/testsuite/ld-i386/pr18329.s    | 17 +++++++++++++++++
 ld/testsuite/ld-i386/pr18329a.d   | 10 ++++++++++
 ld/testsuite/ld-i386/pr18329b.d   |  9 +++++++++
 ld/testsuite/ld-x86-64/pr18329.s  | 17 +++++++++++++++++
 ld/testsuite/ld-x86-64/pr18329a.d | 10 ++++++++++
 ld/testsuite/ld-x86-64/pr18329b.d |  9 +++++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  2 ++
 13 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 ld/testsuite/ld-i386/pr18329.s
 create mode 100644 ld/testsuite/ld-i386/pr18329a.d
 create mode 100644 ld/testsuite/ld-i386/pr18329b.d
 create mode 100644 ld/testsuite/ld-x86-64/pr18329.s
 create mode 100644 ld/testsuite/ld-x86-64/pr18329a.d
 create mode 100644 ld/testsuite/ld-x86-64/pr18329b.d

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index e435e52..cef6573 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1367,6 +1367,10 @@ struct elf_backend_data
   /* Address of protected data defined in the shared library may be
      external, i.e., due to copy relocation.   */
   unsigned extern_protected_data : 1;
+
+  /* True if there are no dynamic relocations against protected functions
+     and variables when they are bound locally.  */
+  unsigned no_dyn_reloc_with_symbolic : 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 d76addb..c60f655 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -5324,6 +5324,7 @@ elf_i386_add_symbol_hook (bfd * abfd,
 #define elf_backend_got_header_size	12
 #define elf_backend_plt_alignment	4
 #define elf_backend_extern_protected_data 1
+#define elf_backend_no_dyn_reloc_with_symbolic 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 a3604c7..d16a1d2 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -5900,6 +5900,7 @@ static const struct bfd_elf_special_section
 #define elf_backend_rela_normal		    1
 #define elf_backend_plt_alignment           4
 #define elf_backend_extern_protected_data   1
+#define elf_backend_no_dyn_reloc_with_symbolic 1
 
 #define elf_info_to_howto		    elf_x86_64_info_to_howto
 
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 6efe1e4..0a782cd 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -8973,6 +8973,12 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
     sym.st_info = ELF_ST_INFO (STB_WEAK, h->type);
   else
     sym.st_info = ELF_ST_INFO (STB_GLOBAL, h->type);
+  /* If backend doesn't generate dynamic relocations against protected
+     functions and variables when they are bound locally, turn off
+     protected visibility on them.  */
+  if (bed->no_dyn_reloc_with_symbolic
+      && SYMBOLIC_BIND (flinfo->info, h))
+    sym.st_other &= ~ELF_ST_VISIBILITY (STV_PROTECTED);
   sym.st_target_internal = h->target_internal;
 
   switch (h->root.type)
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index 0085d6c..27a2c80 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -120,6 +120,9 @@
 #ifndef elf_backend_extern_protected_data
 #define elf_backend_extern_protected_data 0
 #endif
+#ifndef elf_backend_no_dyn_reloc_with_symbolic
+#define elf_backend_no_dyn_reloc_with_symbolic 0
+#endif
 #ifndef elf_backend_stack_align
 #define elf_backend_stack_align 16
 #endif
@@ -810,7 +813,8 @@ static struct elf_backend_data elfNN_bed =
   elf_backend_want_p_paddr_set_to_zero,
   elf_backend_default_execstack,
   elf_backend_caches_rawsize,
-  elf_backend_extern_protected_data
+  elf_backend_extern_protected_data,
+  elf_backend_no_dyn_reloc_with_symbolic
 };
 
 /* Forward declaration for use when initialising alternative_target field.  */
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 8399cbc..cbbc77f 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -255,6 +255,8 @@ run_dump_test "lea1e"
 run_dump_test "lea1f"
 run_dump_test "mov1a"
 run_dump_test "mov1b"
+run_dump_test "pr18329a"
+run_dump_test "pr18329b"
 
 if { !([istarget "i?86-*-linux*"]
        || [istarget "i?86-*-gnu*"]
diff --git a/ld/testsuite/ld-i386/pr18329.s b/ld/testsuite/ld-i386/pr18329.s
new file mode 100644
index 0000000..8262a21
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr18329.s
@@ -0,0 +1,17 @@
+	.text
+	.globl	bar
+	.type	bar, @function
+bar:
+	movl	$30, xxxxx@GOTOFF(%ecx)
+	jmp	yyyyy
+	.globl	yyyyy
+	.protected yyyyy
+	.type	yyyyy, @function
+yyyyy:
+	ret
+	.data
+	.globl xxxxx
+	.type   xxxxx, @object
+	.protected	xxxxx
+xxxxx:
+	.long yyyyy
diff --git a/ld/testsuite/ld-i386/pr18329a.d b/ld/testsuite/ld-i386/pr18329a.d
new file mode 100644
index 0000000..c1bfbba
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr18329a.d
@@ -0,0 +1,10 @@
+#source: pr18329.s
+#ld: -melf_i386 -shared -Bsymbolic
+#as: --32
+#readelf: --wide --dyn-syms
+
+#...
+ +[0-9]+: +[0-9a-f]+ +[0-9]+ +OBJECT +GLOBAL +DEFAULT +[1-9]+ xxxxx
+#...
+ +[0-9]+: +[0-9a-f]+ +[0-9]+ +FUNC +GLOBAL +DEFAULT +[1-9]+ yyyyy
+#pass
diff --git a/ld/testsuite/ld-i386/pr18329b.d b/ld/testsuite/ld-i386/pr18329b.d
new file mode 100644
index 0000000..c449b30
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr18329b.d
@@ -0,0 +1,9 @@
+#source: pr18329.s
+#ld: -melf_i386 -shared -Bsymbolic
+#as: --32
+#readelf: -r
+
+#failif
+#...
+.*[ 	]+(xxxxx|yyyyy).*
+#...
diff --git a/ld/testsuite/ld-x86-64/pr18329.s b/ld/testsuite/ld-x86-64/pr18329.s
new file mode 100644
index 0000000..23c7225
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18329.s
@@ -0,0 +1,17 @@
+	.text
+	.globl	bar
+	.type	bar, @function
+bar:
+	movl	$30, xxxxx(%rip)
+	jmp	yyyyy
+	.globl	yyyyy
+	.protected yyyyy
+	.type	yyyyy, @function
+yyyyy:
+	ret
+	.data
+	.globl xxxxx
+	.type   xxxxx, @object
+	.protected	xxxxx
+xxxxx:
+	.quad	yyyyy
diff --git a/ld/testsuite/ld-x86-64/pr18329a.d b/ld/testsuite/ld-x86-64/pr18329a.d
new file mode 100644
index 0000000..612fd33
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18329a.d
@@ -0,0 +1,10 @@
+#source: pr18329.s
+#ld: -melf_x86_64 -shared -Bsymbolic
+#as: --64
+#readelf: --wide --dyn-syms
+
+#...
+ +[0-9]+: +[0-9a-f]+ +[0-9]+ +OBJECT +GLOBAL +DEFAULT +[1-9]+ xxxxx
+#...
+ +[0-9]+: +[0-9a-f]+ +[0-9]+ +FUNC +GLOBAL +DEFAULT +[1-9]+ yyyyy
+#pass
diff --git a/ld/testsuite/ld-x86-64/pr18329b.d b/ld/testsuite/ld-x86-64/pr18329b.d
new file mode 100644
index 0000000..ea60c2d
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr18329b.d
@@ -0,0 +1,9 @@
+#source: pr18329.s
+#ld: -melf_x86_64 -shared -Bsymbolic
+#as: --64
+#readelf: -r
+
+#failif
+#...
+.*[ 	]+(xxxxx|yyyyy).*
+#...
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 8352ad9..ba8786b 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -239,6 +239,8 @@ run_dump_test "pr14215"
 run_dump_test "pr14207"
 run_dump_test "gotplt1"
 run_dump_test "pie1"
+run_dump_test "pr18329a"
+run_dump_test "pr18329b"
 
 if { ![istarget "x86_64-*-linux*"] && ![istarget "x86_64-*-nacl*"]} {
     return
-- 
2.1.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic
  2015-04-26  0:45   ` H.J. Lu
@ 2015-04-26  1:48     ` Alan Modra
  2015-04-26 13:38       ` H.J. Lu
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2015-04-26  1:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Sat, Apr 25, 2015 at 05:45:13PM -0700, H.J. Lu wrote:
> On Sun, Apr 26, 2015 at 09:16:09AM +0930, Alan Modra wrote:
> > On Sat, Apr 25, 2015 at 02:46:38PM -0700, H.J. Lu wrote:
> > > If a global protected symbol is bound locally by -Bsymbolic, there is no
> > > need to keep protected visibility which hich may have overhead at run-time.
> > 
> > For this change to be safe you'd need to show that all ELF targets
> > supporting dynamic objects also properly support -Bsymbolic for both
> > functions and variables.  ie. that none emit dynamic relocations for
> > the symbol when -Bsymbolic.
> > 
> 
> Good point.  Here is a patch to make it backend dependent.  I will
> check it in shortly.

Before you do that, I'd like to know for sure what it is you're trying
to accomplish.  I'm guessing the "may have overhead at run-time" is
the loop over dynamic relocs in glibc/elf/dl-lookup.c:do_lookup_x.
Correct?

If that's so, then I think you can do better than just clearing the
visibility when -Bsymbolic.  A protected symbol in an executable with
a definition other than in .dynbss, can also avoid the reloc loop.
As in the following testcase.

cat > protected.c <<EOF
int var __attribute__ ((visibility ("protected"))) = 0;

static void __attribute__ ((noinline))
count (void)
{
  var += 1;
}

void
library (void)
{
  int i;
  if (var != 0)
    __builtin_abort ();
  for (i = 0; i < 100000000; i++)
    count ();
}
EOF
cat > protected_add.c <<EOF
int var __attribute__ ((visibility ("protected"))) = 1;
extern void library (void);

int main ()
{
  library ();
  if (var != 1)
    __builtin_abort ();
  return 0;
}
EOF
gcc -O2 -fPIC -shared -o protected.so protected.c
gcc -O2 -fPIC -o protected_app protected_app.c -Wl,-rpath=. protected.so


-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic
  2015-04-26  1:48     ` Alan Modra
@ 2015-04-26 13:38       ` H.J. Lu
  0 siblings, 0 replies; 5+ messages in thread
From: H.J. Lu @ 2015-04-26 13:38 UTC (permalink / raw)
  To: Binutils

On Sat, Apr 25, 2015 at 6:48 PM, Alan Modra <amodra@gmail.com> wrote:
> On Sat, Apr 25, 2015 at 05:45:13PM -0700, H.J. Lu wrote:
>> On Sun, Apr 26, 2015 at 09:16:09AM +0930, Alan Modra wrote:
>> > On Sat, Apr 25, 2015 at 02:46:38PM -0700, H.J. Lu wrote:
>> > > If a global protected symbol is bound locally by -Bsymbolic, there is no
>> > > need to keep protected visibility which hich may have overhead at run-time.
>> >
>> > For this change to be safe you'd need to show that all ELF targets
>> > supporting dynamic objects also properly support -Bsymbolic for both
>> > functions and variables.  ie. that none emit dynamic relocations for
>> > the symbol when -Bsymbolic.
>> >
>>
>> Good point.  Here is a patch to make it backend dependent.  I will
>> check it in shortly.
>
> Before you do that, I'd like to know for sure what it is you're trying
> to accomplish.  I'm guessing the "may have overhead at run-time" is
> the loop over dynamic relocs in glibc/elf/dl-lookup.c:do_lookup_x.
> Correct?

I withdrew my patch.  There is no need to clear PROTECTED visibility
since there is no dynamic relocation against protected symbols in
DSOes built with -Bsymbolic.


-- 
H.J.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-26 13:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-25 21:46 [PATCH] PR ld/18329: Turn off PROTECTED visibility with -Bsymbolic H.J. Lu
2015-04-25 23:46 ` Alan Modra
2015-04-26  0:45   ` H.J. Lu
2015-04-26  1:48     ` Alan Modra
2015-04-26 13:38       ` H.J. Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).