diff mbox

[x86_64] Optimize access to globals in "-fpie -pie" builds with copy relocations

Message ID CAMe9rOpXupztWPgTWAxGx7yP7aynAjYoJbh-5L+wJ3S=qk8CvQ@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Dec. 3, 2014, 9:35 p.m. UTC 
On Wed, Dec 3, 2014 at 7:01 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Dec 3, 2014 at 5:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Dec 2, 2014 at 12:01 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Tue, Dec 2, 2014 at 8:40 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Tue, Dec 2, 2014 at 11:19 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>> Hello!
>>>>>
>>>>>> Ping.
>>>>>>> Ping.
>>>>>>>> Ping.
>>>>>>>>> Ping.
>>>>>
>>>>> It would probably help reviewers if you pointed to actual path
>>>>> submission [1], which unfortunately contains the explanation in the
>>>>> patch itself [2], which further explains that this functionality is
>>>>> currently only supported with gold, patched with [3].
>>>>>
>>>>> [1] https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00645.html
>>>>> [2] https://gcc.gnu.org/ml/gcc-patches/2014-09/txt2CHtu81P1O.txt
>>>>> [3] https://sourceware.org/ml/binutils/2014-05/msg00092.html
>>>>>
>>>>> After a bit of the above detective work, I think that new gcc option
>>>>> is not necessary. The configure should detect if new functionality is
>>>>> supported in the linker, and auto-configure gcc to use it when
>>>>> appropriate.
>>>>
>>>> I think GCC option is needed since one can use -fuse-ld= to
>>>> change linker.
>>>
>>> IMO, nobody will use this highly special x86_64-only option. It would
>>> be best for gnu-ld to reach feature parity with gold as far as this
>>> functionality is concerned. In this case, the optimization would be
>>> auto-configured, and would fire automatically, without any user
>>> intervention.
>>>
>>
>> Let's do it.  I implemented the same feature in bfd linker on both
>> master and 2.25 branch.
>>
>
> +bool
> +i386_binds_local_p (const_tree exp)
> +{
> +  /* Globals marked extern are treated as local when linker copy relocations
> +     support is available with -f{pie|PIE}.  */
> +  if (TARGET_64BIT && ix86_copyrelocs && flag_pie
> +      && TREE_CODE (exp) == VAR_DECL
> +      && DECL_EXTERNAL (exp) && !DECL_WEAK (exp))
> +    return true;
> +  return default_binds_local_p (exp);
> +}
> +
>
> It returns true with -fPIE and false without -fPIE.  It is lying to compiler.
> Maybe legitimate_pic_address_disp_p is a better place.
>

Something like this?

Comments

Uros Bizjak Dec. 4, 2014, 12:44 p.m. UTC | #1 
On Wed, Dec 3, 2014 at 10:35 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>>>>> It would probably help reviewers if you pointed to actual path
>>>>>> submission [1], which unfortunately contains the explanation in the
>>>>>> patch itself [2], which further explains that this functionality is
>>>>>> currently only supported with gold, patched with [3].
>>>>>>
>>>>>> [1] https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00645.html
>>>>>> [2] https://gcc.gnu.org/ml/gcc-patches/2014-09/txt2CHtu81P1O.txt
>>>>>> [3] https://sourceware.org/ml/binutils/2014-05/msg00092.html
>>>>>>
>>>>>> After a bit of the above detective work, I think that new gcc option
>>>>>> is not necessary. The configure should detect if new functionality is
>>>>>> supported in the linker, and auto-configure gcc to use it when
>>>>>> appropriate.
>>>>>
>>>>> I think GCC option is needed since one can use -fuse-ld= to
>>>>> change linker.
>>>>
>>>> IMO, nobody will use this highly special x86_64-only option. It would
>>>> be best for gnu-ld to reach feature parity with gold as far as this
>>>> functionality is concerned. In this case, the optimization would be
>>>> auto-configured, and would fire automatically, without any user
>>>> intervention.
>>>>
>>>
>>> Let's do it.  I implemented the same feature in bfd linker on both
>>> master and 2.25 branch.
>>>
>>
>> +bool
>> +i386_binds_local_p (const_tree exp)
>> +{
>> +  /* Globals marked extern are treated as local when linker copy relocations
>> +     support is available with -f{pie|PIE}.  */
>> +  if (TARGET_64BIT && ix86_copyrelocs && flag_pie
>> +      && TREE_CODE (exp) == VAR_DECL
>> +      && DECL_EXTERNAL (exp) && !DECL_WEAK (exp))
>> +    return true;
>> +  return default_binds_local_p (exp);
>> +}
>> +
>>
>> It returns true with -fPIE and false without -fPIE.  It is lying to compiler.
>> Maybe legitimate_pic_address_disp_p is a better place.

Agreed.

> Something like this?

Yes.

OK, if Jakub doesn't have any objections here. Please also add
Sriraman as author to ChangeLog entry.

Thanks,
Uros.
diff mbox

Patch

diff --git a/gcc/config.in b/gcc/config.in
index 65d5e42..f34adb5 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -1411,6 +1411,12 @@ 
 #endif
 
 
+/* Define 0/1 if your linker supports -pie option with copy reloc. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_LD_PIE_COPYRELOC
+#endif
+
+
 /* Define if your linker links a mix of read-only and read-write sections into
    a read-write section. */
 #ifndef USED_FOR_TARGET
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 211c9e6..eb43bc6 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13113,7 +13113,10 @@  legitimate_pic_address_disp_p (rtx disp)
 		return true;
 	    }
 	  else if (!SYMBOL_REF_FAR_ADDR_P (op0)
-		   && SYMBOL_REF_LOCAL_P (op0)
+		   && (SYMBOL_REF_LOCAL_P (op0)
+		       || (HAVE_LD_PIE_COPYRELOC
+			   && flag_pie
+			   && !SYMBOL_REF_FUNCTION_P (op0)))
 		   && ix86_cmodel != CM_LARGE_PIC)
 	    return true;
 	  break;
diff --git a/gcc/configure b/gcc/configure
index 6b46bbb..811f05d 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -27025,6 +27025,53 @@  fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_ld_pie" >&5
 $as_echo "$gcc_cv_ld_pie" >&6; }
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking linker PIE support with copy reloc" >&5
+$as_echo_n "checking linker PIE support with copy reloc... " >&6; }
+gcc_cv_ld_pie_copyreloc=no
+if test $gcc_cv_ld_pie = yes ; then
+  if test $in_tree_ld = yes ; then
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt 2; then
+      gcc_cv_ld_pie_copyreloc=yes
+    fi
+  elif test x$gcc_cv_as != x -a x$gcc_cv_ld != x ; then
+    # Check if linker supports -pie option with copy reloc
+    case "$target" in
+    i?86-*-linux* | x86_64-*-linux*)
+      cat > conftest1.s <<EOF
+	.globl	a_glob
+	.data
+	.type	a_glob, @object
+	.size	a_glob, 4
+a_glob:
+	.long	2
+EOF
+      cat > conftest2.s <<EOF
+	.text
+	.globl	main
+	.type	main, @function
+main:
+	movl	%eax, a_glob(%rip)
+	.size	main, .-main
+EOF
+      if $gcc_cv_as --64 -o conftest1.o conftest1.s > /dev/null 2>&1 \
+         && $gcc_cv_ld -shared -melf_x86_64 -o conftest1.so conftest1.o > /dev/null 2>&1 \
+         && $gcc_cv_as --64 -o conftest2.o conftest2.s > /dev/null 2>&1 \
+         && $gcc_cv_ld -pie -melf_x86_64 -o conftest conftest2.o conftest1.so > /dev/null 2>&1; then
+        gcc_cv_ld_pie_copyreloc=yes
+      fi
+      rm -f conftest conftest1.so conftest1.o conftest2.o conftest1.s conftest2.s
+      ;;
+    esac
+  fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_LD_PIE_COPYRELOC `if test x"$gcc_cv_ld_pie_copyreloc" = xyes; then echo 1; else echo 0; fi`
+_ACEOF
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gcc_cv_ld_pie_copyreloc" >&5
+$as_echo "$gcc_cv_ld_pie_copyreloc" >&6; }
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking linker EH-compatible garbage collection of sections" >&5
 $as_echo_n "checking linker EH-compatible garbage collection of sections... " >&6; }
 gcc_cv_ld_eh_gc_sections=no
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 48c8000..a33f3a5 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4693,6 +4693,49 @@  if test x"$gcc_cv_ld_pie" = xyes; then
 fi
 AC_MSG_RESULT($gcc_cv_ld_pie)
 
+AC_MSG_CHECKING(linker PIE support with copy reloc)
+gcc_cv_ld_pie_copyreloc=no
+if test $gcc_cv_ld_pie = yes ; then
+  if test $in_tree_ld = yes ; then
+    if test "$gcc_cv_gld_major_version" -eq 2 -a "$gcc_cv_gld_minor_version" -ge 25 -o "$gcc_cv_gld_major_version" -gt 2; then
+      gcc_cv_ld_pie_copyreloc=yes
+    fi
+  elif test x$gcc_cv_as != x -a x$gcc_cv_ld != x ; then
+    # Check if linker supports -pie option with copy reloc
+    case "$target" in
+    i?86-*-linux* | x86_64-*-linux*)
+      cat > conftest1.s <<EOF
+	.globl	a_glob
+	.data
+	.type	a_glob, @object
+	.size	a_glob, 4
+a_glob:
+	.long	2
+EOF
+      cat > conftest2.s <<EOF
+	.text
+	.globl	main
+	.type	main, @function
+main:
+	movl	%eax, a_glob(%rip)
+	.size	main, .-main
+EOF
+      if $gcc_cv_as --64 -o conftest1.o conftest1.s > /dev/null 2>&1 \
+         && $gcc_cv_ld -shared -melf_x86_64 -o conftest1.so conftest1.o > /dev/null 2>&1 \
+         && $gcc_cv_as --64 -o conftest2.o conftest2.s > /dev/null 2>&1 \
+         && $gcc_cv_ld -pie -melf_x86_64 -o conftest conftest2.o conftest1.so > /dev/null 2>&1; then
+        gcc_cv_ld_pie_copyreloc=yes
+      fi
+      rm -f conftest conftest1.so conftest1.o conftest2.o conftest1.s conftest2.s
+      ;;
+    esac
+  fi
+  AC_DEFINE_UNQUOTED(HAVE_LD_PIE_COPYRELOC,
+    [`if test x"$gcc_cv_ld_pie_copyreloc" = xyes; then echo 1; else echo 0; fi`],
+    [Define 0/1 if your linker supports -pie option with copy reloc.])
+fi
+AC_MSG_RESULT($gcc_cv_ld_pie_copyreloc)
+
 AC_MSG_CHECKING(linker EH-compatible garbage collection of sections)
 gcc_cv_ld_eh_gc_sections=no
 if test $in_tree_ld = yes ; then