diff mbox

PR target/65248: [5 Regression] Copy relocation in PIE against protected symbol

Message ID CAMe9rOpCzX5fzKduoezC-mOxd=zo=B=NnLOuLWLC-9nv+DZ7Ng@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu March 1, 2015, 3:33 p.m. UTC
On Sat, Feb 28, 2015 at 5:27 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> On 02/28/2015 09:42 AM, H.J. Lu wrote:
>>
>> @@ -22704,6 +22705,13 @@ For systems that use the GNU C Library, the
>> default is on.
>>   Specify that the assembler should encode SSE instructions with VEX
>>   prefix.  The option @option{-mavx} turns this on by default.
>>
>> +@item -mcopyreloc-in-pie
>> +@itemx -mno-copyreloc-in-pie
>> +@opindex mcopyreloc-in-pie
>> +Use copy relocations in Position Independent Executable (PIE).  It
>> +requires linker support.  This option is turned on by default if linker
>> +used to build GCC supports it.
>> +
>
>
> How about:  "...if GCC is configured to use a linker that supports these
> relocations."
>
> I assume this is a property of the target linker, and not literally the host
> linker used to build GCC?

Yes, that is correct.  Here is the updated patch.

Thanks.

Comments

Alan Modra March 2, 2015, 7:30 a.m. UTC | #1
On Sun, Mar 01, 2015 at 07:33:14AM -0800, H.J. Lu wrote:
> +mcopyreloc-in-pie

I'm not calling for a change in the name of the option (*), but
technically it isn't completely correct to call your optimisation
"copy reloc in pie".  What you are really doing is using a linker
generated variable (in .dynbss of an executable), in place of a
variable definition in a shared library.  Not all such variables use
copy relocs!  So it is quite possible for your optimisation to trigger
but there be no sign of an R_<machine>_COPY relocation in the final
executable.

I do think your documentation should be talking about .dynbss copies
of variables rather than copy relocations, which are just a way of
initializing such variables.


*) I should have fixed the name of the linker option added here:
https://sourceware.org/ml/binutils/2001-09/msg00506.html
diff mbox

Patch

From 91de4b11eb2d7f9b0abd83837deb510a6f69cd81 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sat, 28 Feb 2015 08:31:36 -0800
Subject: [PATCH] Add -mcopyreloc-in-pie

Ue copy relocation in PIE improves performance.  But copy relocation
can't be used to access protected symbols defined in shared libaries
and linker in binutils 2.26 enforces doesn't allow it.  GCC doesn't
know if an external definition is protected or not.  This option gives
user an option to turn it off to avoid problem at link-time.

gcc/

	PR target/65248
	* config/i386/i386.c (ix86_option_override_internal): Set
	flag_copyreloc_in_pie to HAVE_LD_PIE_COPYRELOC if not set.
	(legitimate_pic_address_disp_p): Replace HAVE_LD_PIE_COPYRELOC
	with flag_copyreloc_in_pie.
	* config/i386/i386.opt (mcopyreloc-in-pie): New.
	* doc/invoke.texi: Document -mcopyreloc-in-pie.

gcc/testsuite/

	PR target/65248
	* gcc.target/i386/pr65248-1.c: New.
	* gcc.target/i386/pr65248-2.c: Likewise.
	* gcc.target/i386/pr65248-3.c: Likewise.
	* gcc.target/i386/pr65248-4.c: Likewise.
---
 gcc/config/i386/i386.c                    |  6 +++++-
 gcc/config/i386/i386.opt                  |  4 ++++
 gcc/doc/invoke.texi                       | 10 +++++++++-
 gcc/testsuite/gcc.target/i386/pr65248-1.c | 13 +++++++++++++
 gcc/testsuite/gcc.target/i386/pr65248-2.c | 13 +++++++++++++
 gcc/testsuite/gcc.target/i386/pr65248-3.c | 16 ++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr65248-4.c | 16 ++++++++++++++++
 7 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr65248-4.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index bec1324..6768ee8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -4248,6 +4248,10 @@  ix86_option_override_internal (bool main_args_p,
 #endif
    }
 
+  /* Set the default value for -mcopyreloc-in-pie.  */
+  if (opts->x_flag_copyreloc_in_pie == -1)
+    opts->x_flag_copyreloc_in_pie = HAVE_LD_PIE_COPYRELOC;
+
   if (!(opts_set->x_target_flags & MASK_VZEROUPPER))
     opts->x_target_flags |= MASK_VZEROUPPER;
   if (!ix86_tune_features[X86_TUNE_AVX256_UNALIGNED_LOAD_OPTIMAL]
@@ -13230,7 +13234,7 @@  legitimate_pic_address_disp_p (rtx disp)
 	    }
 	  else if (!SYMBOL_REF_FAR_ADDR_P (op0)
 		   && (SYMBOL_REF_LOCAL_P (op0)
-		       || (HAVE_LD_PIE_COPYRELOC
+		       || (flag_copyreloc_in_pie
 			   && flag_pie
 			   && !SYMBOL_REF_WEAK (op0)
 			   && !SYMBOL_REF_FUNCTION_P (op0)))
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 301430c..55c712c 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -826,6 +826,10 @@  mfentry
 Target Report Var(flag_fentry) Init(-1)
 Emit profiling counter call at function entry before prologue.
 
+mcopyreloc-in-pie
+Target Report Var(flag_copyreloc_in_pie) Init(-1)
+Use copy relocations in Position Independent Executable (PIE)
+
 mrecord-mcount
 Target Report Var(flag_record_mcount) Init(0)
 Generate __mcount_loc section with all mcount or __fentry__ calls.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index a87376e..de228c4 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1078,7 +1078,8 @@  See RS/6000 and PowerPC Options.
 -m32 -m64 -mx32 -m16 -mlarge-data-threshold=@var{num} @gol
 -msse2avx -mfentry -mrecord-mcount -mnop-mcount -m8bit-idiv @gol
 -mavx256-split-unaligned-load -mavx256-split-unaligned-store @gol
--malign-data=@var{type} -mstack-protector-guard=@var{guard}}
+-malign-data=@var{type} -mstack-protector-guard=@var{guard}} @gol
+-mcopyreloc-in-pie
 
 @emph{x86 Windows Options}
 @gccoptlist{-mconsole -mcygwin -mno-cygwin -mdll @gol
@@ -22704,6 +22705,13 @@  For systems that use the GNU C Library, the default is on.
 Specify that the assembler should encode SSE instructions with VEX
 prefix.  The option @option{-mavx} turns this on by default.
 
+@item -mcopyreloc-in-pie
+@itemx -mno-copyreloc-in-pie
+@opindex mcopyreloc-in-pie
+Use copy relocations in Position Independent Executable (PIE).  It
+requires linker support.  This option is turned on by default if GCC
+is configured to use a linker that supports copy relocations in PIE.
+
 @item -mfentry
 @itemx -mno-fentry
 @opindex mfentry
diff --git a/gcc/testsuite/gcc.target/i386/pr65248-1.c b/gcc/testsuite/gcc.target/i386/pr65248-1.c
new file mode 100644
index 0000000..91aa6be
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65248-1.c
@@ -0,0 +1,13 @@ 
+/* Check that GOTPCREL isn't used to access glob_a.  */
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie -mcopyreloc-in-pie" } */
+
+extern int glob_a;
+
+int foo ()
+{
+  return glob_a;
+}
+
+/* glob_a should never be accessed with a GOTPCREL.  */
+/* { dg-final { scan-assembler-not "glob_a@GOTPCREL" { target { ! ia32 } } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr65248-2.c b/gcc/testsuite/gcc.target/i386/pr65248-2.c
new file mode 100644
index 0000000..dc8445c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65248-2.c
@@ -0,0 +1,13 @@ 
+/* Check that GOTPCREL isn't used to access glob_a.  */
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie -mno-copyreloc-in-pie" } */
+
+int glob_a;
+
+int foo ()
+{
+  return glob_a;
+}
+
+/* glob_a should never be accessed with a GOTPCREL.  */
+/* { dg-final { scan-assembler-not "glob_a@GOTPCREL" { target { ! ia32 } } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr65248-3.c b/gcc/testsuite/gcc.target/i386/pr65248-3.c
new file mode 100644
index 0000000..9398fd2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65248-3.c
@@ -0,0 +1,16 @@ 
+/* Check that GOTPCREL is used to access glob_a.  */
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie -mno-copyreloc-in-pie" } */
+
+extern int glob_a;
+
+int foo ()
+{
+  if (&glob_a != 0)
+    return glob_a;
+  else
+    return 0;
+}
+
+/* weak glob_a should be accessed with a GOTPCREL.  */
+/* { dg-final { scan-assembler "glob_a@GOTPCREL" { target { ! ia32 } } } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr65248-4.c b/gcc/testsuite/gcc.target/i386/pr65248-4.c
new file mode 100644
index 0000000..6d7cc86
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr65248-4.c
@@ -0,0 +1,16 @@ 
+/* Check that GOTPCREL is used to access glob_a.  */
+/* { dg-do compile { target *-*-linux* } } */
+/* { dg-options "-O2 -fpie -mno-copyreloc-in-pie" } */
+
+extern int glob_a  __attribute__((weak));
+
+int foo ()
+{
+  if (&glob_a != 0)
+    return glob_a;
+  else
+    return 0;
+}
+
+/* weak glob_a should be accessed with a GOTPCREL.  */
+/* { dg-final { scan-assembler "glob_a@GOTPCREL" { target { ! ia32 } } } } */
-- 
2.1.0